Skip to content
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

Global CLI flag should override env var flag #10423

Merged
merged 3 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20240709-172440.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: CLI flags should take precedence over env var flags
time: 2024-07-09T17:24:40.918977-04:00
custom:
Author: gshank
Issue: "10304"
22 changes: 20 additions & 2 deletions core/dbt/cli/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ def __init__(
# Set the default flags.
for key, value in FLAGS_DEFAULTS.items():
object.__setattr__(self, key, value)
# Use to handle duplicate params in _assign_params
flags_defaults_list = list(FLAGS_DEFAULTS.keys())

if ctx is None:
ctx = get_current_context()
Expand Down Expand Up @@ -173,13 +175,29 @@ def _assign_params(
old_name=dep_param.envvar,
new_name=new_param.envvar,
)
# end deprecated_params

# Set the flag value.
is_duplicate = hasattr(self, param_name.upper())
is_duplicate = (
hasattr(self, param_name.upper())
and param_name.upper() not in flags_defaults_list
)
# First time through, set as though FLAGS_DEFAULTS hasn't been set, so not a duplicate.
# Subsequent pass (to process "parent" params) should be treated as duplicates.
if param_name.upper() in flags_defaults_list:
flags_defaults_list.remove(param_name.upper())
# Note: the following determines whether parameter came from click default,
# not from FLAGS_DEFAULTS in __init__.
is_default = ctx.get_parameter_source(param_name) == ParameterSource.DEFAULT
is_envvar = ctx.get_parameter_source(param_name) == ParameterSource.ENVIRONMENT

flag_name = (new_name or param_name).upper()

if (is_duplicate and not is_default) or not is_duplicate:
# envvar flags are assigned in either parent or child context if there
# isn't an overriding cli command flag.
# If the flag has been encountered as a child cli flag, we don't
# want to overwrite with parent envvar, since the commandline flag takes precedence.
if (is_duplicate and not (is_default or is_envvar)) or not is_duplicate:
object.__setattr__(self, flag_name, param_value)

# Track default assigned params.
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/cli/test_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,22 @@ def test_global_flag_at_child_context(self):

assert flags_a.USE_COLORS == flags_b.USE_COLORS

def test_global_flag_with_env_var(self, monkeypatch):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this test

# The environment variable is used for whichever parent or child
# does not have a cli command.
# Test that "child" global flag overrides env var
monkeypatch.setenv("DBT_QUIET", "0")
parent_context = self.make_dbt_context("parent", ["--no-use-colors"])
child_context = self.make_dbt_context("child", ["--quiet"], parent_context)
flags = Flags(child_context)
assert flags.QUIET is True

# Test that "parent" global flag overrides env var
parent_context = self.make_dbt_context("parent", ["--quiet"])
child_context = self.make_dbt_context("child", ["--no-use-colors"], parent_context)
flags = Flags(child_context)
assert flags.QUIET is True

def test_set_project_only_flags(self, project_flags, run_context):
flags = Flags(run_context, project_flags)

Expand Down
Loading