Fix some issues around global-scoped vars #2477
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
Fix global-level var rewriting from v2 -> v1 Add top-level vars to more lookup fields When a user calls var() in a `generate_*_name` macro, on failure it now raises (like at runtime) When a node has no FQN, pretend the FQN is just the package name when returning vars Add tests
Change variables named `l` to `log` Make IsFQNResource runtime type-checkable and use that to handle the FQN check Make the GenerateNameProvider its own top-level Provider type
drewbanin
reviewed
May 20, 2020
|
|
||
| project_type_vars.update({ | ||
| k: v for k, v in src_vars.items() | ||
| if not isinstance(v, dict) |
There was a problem hiding this comment.
what's the thinking here? Is it that we don't want to provide package-scoped vars to the wrong package? It also means that you can't provide a global var that is a dict to a v1 package, right?
I'm curious what the ramifications would be if we removed this filter and added all of the src_vars to the v1 package's vars
There was a problem hiding this comment.
Yeah. We can remove that if not isinstance, I thought it made the debug dictionary really a pain.
There was a problem hiding this comment.
ok, that's fine - let's ship it as-is and I'll be sure to update the docs accordingly
kwigley
approved these changes
May 20, 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.
resolves #2469
resolves #2473
resolves #2472
Description
Fix global-level var rewriting from v2 -> v1
+syntax in the future.Add top-level vars to more lookup fields
vars, and a couple only handled the package-scoped ones. I believe I've resolved all of them.When a user calls var() in a
generate_*_namemacro, on failure it now raises (like at runtime)varandconfigact as if it were runtime. Maybeconfig.getjust should not work at all?When a node has no FQN, pretend the FQN is just the package name when returning vars
generate_*_namemacros would somehow resolve vars against thenodethey're passed instead of the context's node. That's a much more complicated problem that probably deserves its own issue/PR process!Checklist
CHANGELOG.mdand added information about my change to the "dbt next" section.