-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework dispatch, with backwards compatibility for current use #3363
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirmed that this offers backwards compatibility for the dbt-utils
package. Hooray! I moved a little of the logic around, to account for when packages
is a positional arg and also a Call
, and I touched up the error message. (Looks like I messed up the flake8 linting, sorry about that.)
I'm so happy to be adding this logic and ensure projects have a smooth upgrade path to v0.19.2 + v0.20.0; I also want to get "the right way" to do this in place, and immediately recommend it to everyone who's upgrading. I laid out my sketch of a longer-term sustainable solution over in #3383.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes are looking really good! I confirmed they worked with dbt-utils
, both with:
- The setup as it exists today
- The changes we'll want to make to support the new
dispatch
project config
I left a few comments, nothing major.
Next steps from here:
- Merge this PR into
develop
(or a new one, it doesn't have to be the one I stubbed out last week! haha) - Cherry-pick this PR onto
0.19.latest
- Create a new PR, for inclusion in
develop
(v0.20) only, that raises a deprecation warning if the user either provides thepackages
kwarg or passes a list to the second positional arg
"dispatch": [ | ||
{ | ||
"macro_namespace": "test_utils", | ||
"search_order": ['test_utils'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we adjust this test, or add another one, to confirm that when a user sets this to
"search_order": ['another_package', 'test_utils']
dbt will indeed prefer the version of the macro defined in another_package
? Ultimately, that's the dispatch
functionality that we want to be safeguarding here.
used in parsing schema tests by looking at the arguments to adapter.dispatch. Includes providing an alternative way of specifying macro search order in project config. Collaboratively developed with Jeremy Cohen.
…be (#3363) used in parsing schema tests by looking at the arguments to adapter.dispatch. Includes providing an alternative way of specifying macro search order in project config. Collaboratively developed with Jeremy Cohen. Co-authored-by: Gerda Shank <gerda@fishtownanalytics.com>
…be (#3363) used in parsing schema tests by looking at the arguments to adapter.dispatch. Includes providing an alternative way of specifying macro search order in project config. Collaboratively developed with Jeremy Cohen. Co-authored-by: Gerda Shank <gerda@fishtownanalytics.com>
…be (#3363) used in parsing schema tests by looking at the arguments to adapter.dispatch. Includes providing an alternative way of specifying macro search order in project config. Collaboratively developed with Jeremy Cohen. Co-authored-by: Gerda Shank <gerda@fishtownanalytics.com>
…be (#3363) used in parsing schema tests by looking at the arguments to adapter.dispatch. Includes providing an alternative way of specifying macro search order in project config. Collaboratively developed with Jeremy Cohen. Co-authored-by: Gerda Shank <gerda@fishtownanalytics.com>
…be (#3363) used in parsing schema tests by looking at the arguments to adapter.dispatch. Includes providing an alternative way of specifying macro search order in project config. Collaboratively developed with Jeremy Cohen. Co-authored-by: Gerda Shank <gerda@fishtownanalytics.com>
…be (#3363) used in parsing schema tests by looking at the arguments to adapter.dispatch. Includes providing an alternative way of specifying macro search order in project config. Collaboratively developed with Jeremy Cohen. Co-authored-by: Gerda Shank <gerda@fishtownanalytics.com> automatic commit by git-black, original commits: 98c015b
resolves #3362
resolves #3383
This just adds a reproduction case for now. I've confirmed that, when I run pytest locally, it raises the same error we're seeing in the wild:
Checklist
CHANGELOG.md
and added information about my change to the "dbt next" section.