-
Notifications
You must be signed in to change notification settings - Fork 35
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
[Bug] Behavior flags should take the default value when the project file is not loaded #338
base: main
Are you sure you want to change the base?
Conversation
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
] | ||
|
||
|
||
def test_register_behavior_flags(adapter): |
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.
This is the same test as in ./test_behavior_flags.py
, but with flags
removed from the config
fixture above, and swapping the expected values for the two cases where the user override changes the result.
# we don't always get project flags, for example, the project file is not loaded during `dbt debug` | ||
# in that case, load the default values for behavior flags to avoid compilation errors | ||
# this mimics not loading a project file, or not specifying flags in a project file | ||
user_overrides = getattr(self.config, "flags", {}) |
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.
This is effectively the same as swapping []
for flags
on line 308
, but doing this reads much more cleanly.
Problem
We don't always get the
flags
property on theconfig
object that gets used to create aBaseAdapter
instance. This happens wheneverproject.yml
is not loaded. In particular, this happens duringdbt debug
, which is meant to be a quick check of whether we can connect to a database; this command only requiresprofile.yml
.The decision was initially made to not load any behavior flags in the scenarios where
project.yml
was not loaded. The rationale there was that we did not want to return one value for a behavior flag whenproject.yml
is not loaded (hence the default) and another value for the behavior flag whenproject.yml
is loaded (the default, potentially overridden by the user). However, in the former scenario, referencing the behavior flag at all results in aCompilationError
since the flag does not exist onBaseAdapter.behavior
. This is not desirable behavior either.Solution
While the behavior flags are loaded onto
BaseAdapter.behavior
dynamically, the object should always have the same flags available. Since the user's overrides are not available whenproject.yml
is not loaded, we should act as ifproject.yml
was loaded, but without aflags
node. In other words, we should simply assume the default values when we don't get aflags
attribute.Checklist