Fix manifest search (#2032) #2035
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
beckjake
force-pushed
the
fix/manifest-search
branch
from
02ed1b5 to
90393c1
Compare
|
rebased + fixed merge conflicts |
Unify the lookup for the various macros, all of them are done in the manifest now: - dbt's built-in package + all plugins are part of 'core' - finding macros has the concept of an optional package name - materializations are a special case of finding macros - the `generate_*_name` macros are another special case (non-core dependency packages are ignored) - generating the macro execution context now uses the concept of an optional search_package_name - macro execution now binds these two together Unify the lookup for models/docs/sources - added a search_name property to those three types - create a 'Searchable' protocol, have the node search code accept that - simplify matching logic/push it into the correct types accordingly Rename get_materialization_macro to find_materialization_macro_by_name (consistency) context namespacing behavior: - dbt run-operation now passes the named package along to macro execution if a package name is specified - so `dbt run-operation dependency.ad` runs with the dependency namespace as local, overriding globals - but `dbt run-operation my_macro` runs in the current package namespace as local, even if it ultimately runs the dependency's my_macro() - the current package namespace is always treated as "global", overriding core macros Tons of tests
beckjake
force-pushed
the
fix/manifest-search
branch
from
90393c1 to
62755fe
Compare
drewbanin
reviewed
Jan 31, 2020
| - if neither of those exist (unit tests?), return None | ||
| """ | ||
| def filter(candidate: MacroCandidate) -> bool: | ||
| return candidate.locality != Locality.Imported |
drewbanin
approved these changes
Jan 31, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2032
I went a bit further and also included the node search since it was very easy and I had a nice test suite for it...
Unify the lookup for the various macros, all of them are done in the manifest now:
generate_*_namemacros are another special case (non-core dependency packages are ignored)Unify the lookup for models/docs/sources:
Renamed
get_materialization_macrotofind_materialization_macro_by_name(consistency)context namespacing behavior:
dbt run-operation dependency.adruns with the dependency namespace as local, overriding globalsdbt run-operation my_macroruns in the current package namespace as local, even if it ultimately runs the dependency's my_macro()Added type annotations for all the manifest stuff
Tons of tests