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

Deprecate more env vars #7175

Merged
merged 2 commits into from
Mar 20, 2023
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/Under the Hood-20230315-122723.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Deprecate additional environment variables
time: 2023-03-15T12:27:23.194686-05:00
custom:
Author: stu-k
Issue: "6903"
17 changes: 16 additions & 1 deletion core/dbt/cli/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ def args_to_context(args: List[str]) -> Context:


DEPRECATED_PARAMS = {
"deprecated_defer": "defer",
"deprecated_favor_state": "favor_state",
"deprecated_print": "print",
"deprecated_state": "state",
}


Expand Down Expand Up @@ -132,7 +135,8 @@ def assign_params(ctx, params_assigned_from_default, deprecated_env_vars):
continue
elif param_source != ParameterSource.ENVIRONMENT:
raise BadOptionUsage(
"Deprecated parameters can only be set via environment variables"
param_name,
"Deprecated parameters can only be set via environment variables",
)

# rename for clarity
Expand All @@ -148,6 +152,11 @@ def assign_params(ctx, params_assigned_from_default, deprecated_env_vars):
f"No deprecated param name match from {dep_name} to {new_name}"
)

# remove param from defaulted set since the deprecated
# value is not set from default, but from an env var
if new_name in params_assigned_from_default:
params_assigned_from_default.remove(new_name)

# adding the deprecation warning function to the set
deprecated_env_vars[new_name] = renamed_env_var(
old_name=dep_opt.envvar,
Expand Down Expand Up @@ -259,3 +268,9 @@ def _assert_mutually_exclusive(
)
elif flag_set_by_user:
set_flag = flag

def fire_deprecations(self):
[dep_fn() for dep_fn in self.deprecated_env_var_warnings]
# it is necessary to remove this attr from the class so it does
# not get pickled when written to disk as json
object.__delattr__(self, "deprecated_env_var_warnings")
21 changes: 21 additions & 0 deletions core/dbt/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,11 @@ def cli(ctx, **kwargs):
@cli.command("build")
@click.pass_context
@p.defer
@p.deprecated_defer
@p.exclude
@p.fail_fast
@p.favor_state
@p.deprecated_favor_state
@p.full_refresh
@p.indirect_selection
@p.profile
Expand All @@ -117,6 +119,7 @@ def cli(ctx, **kwargs):
@p.selector
@p.show
@p.state
@p.deprecated_state
@p.store_failures
@p.target
@p.target_path
Expand Down Expand Up @@ -173,14 +176,17 @@ def docs(ctx, **kwargs):
@click.pass_context
@p.compile_docs
@p.defer
@p.deprecated_defer
@p.exclude
@p.favor_state
@p.deprecated_favor_state
@p.profile
@p.profiles_dir
@p.project_dir
@p.select
@p.selector
@p.state
@p.deprecated_state
@p.target
@p.target_path
@p.threads
Expand Down Expand Up @@ -236,8 +242,10 @@ def docs_serve(ctx, **kwargs):
@cli.command("compile")
@click.pass_context
@p.defer
@p.deprecated_defer
@p.exclude
@p.favor_state
@p.deprecated_favor_state
@p.full_refresh
@p.indirect_selection
@p.introspect
Expand All @@ -249,6 +257,7 @@ def docs_serve(ctx, **kwargs):
@p.selector
@p.inline
@p.state
@p.deprecated_state
@p.target
@p.target_path
@p.threads
Expand Down Expand Up @@ -351,6 +360,7 @@ def init(ctx, **kwargs):
@p.raw_select
@p.selector
@p.state
@p.deprecated_state
@p.target
@p.vars
@requires.preflight
Expand Down Expand Up @@ -405,7 +415,9 @@ def parse(ctx, **kwargs):
@cli.command("run")
@click.pass_context
@p.defer
@p.deprecated_defer
@p.favor_state
@p.deprecated_favor_state
@p.exclude
@p.fail_fast
@p.full_refresh
Expand All @@ -415,6 +427,7 @@ def parse(ctx, **kwargs):
@p.select
@p.selector
@p.state
@p.deprecated_state
@p.target
@p.target_path
@p.threads
Expand Down Expand Up @@ -478,6 +491,7 @@ def run_operation(ctx, **kwargs):
@p.selector
@p.show
@p.state
@p.deprecated_state
@p.target
@p.target_path
@p.threads
Expand All @@ -504,14 +518,17 @@ def seed(ctx, **kwargs):
@cli.command("snapshot")
@click.pass_context
@p.defer
@p.deprecated_defer
@p.exclude
@p.favor_state
@p.deprecated_favor_state
@p.profile
@p.profiles_dir
@p.project_dir
@p.select
@p.selector
@p.state
@p.deprecated_state
@p.target
@p.threads
@p.vars
Expand Down Expand Up @@ -551,6 +568,7 @@ def source(ctx, **kwargs):
@p.select
@p.selector
@p.state
@p.deprecated_state
@p.target
@p.threads
@p.vars
Expand Down Expand Up @@ -582,16 +600,19 @@ def freshness(ctx, **kwargs):
@cli.command("test")
@click.pass_context
@p.defer
@p.deprecated_defer
@p.exclude
@p.fail_fast
@p.favor_state
@p.deprecated_favor_state
@p.indirect_selection
@p.profile
@p.profiles_dir
@p.project_dir
@p.select
@p.selector
@p.state
@p.deprecated_state
@p.store_failures
@p.target
@p.target_path
Expand Down
38 changes: 29 additions & 9 deletions core/dbt/cli/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,21 @@
help="Display debug logging during dbt execution. Useful for debugging and making bug reports.",
)

# TODO: The env var and name (reflected in flags) are corrections!
# The original name was `DEFER_MODE` and used an env var called "DBT_DEFER_TO_STATE"
# Both of which break existing naming conventions.
# This will need to be fixed before use in the main codebase and communicated as a change to the community!
# flag was previously named DEFER_MODE
defer = click.option(
"--defer/--no-defer",
envvar="DBT_DEFER",
help="If set, defer to the state variable for resolving unselected nodes.",
)

deprecated_defer = click.option(
"--deprecated-defer",
envvar="DBT_DEFER_TO_STATE",
help="Internal flag for deprecating old env var.",
default=False,
hidden=True,
)

enable_legacy_logger = click.option(
"--enable-legacy-logger/--no-enable-legacy-logger",
envvar="DBT_ENABLE_LEGACY_LOGGER",
Expand All @@ -95,6 +100,12 @@
help="If set, defer to the argument provided to the state flag for resolving unselected nodes, even if the node(s) exist as a database object in the current environment.",
)

deprecated_favor_state = click.option(
"--deprecated-favor-state",
envvar="DBT_FAVOR_STATE_MODE",
help="Internal flag for deprecating old env var.",
)

full_refresh = click.option(
"--full-refresh",
"-f",
Expand Down Expand Up @@ -214,7 +225,6 @@
type=click.INT,
)

# envvar was previously named DBT_NO_PRINT
print = click.option(
"--print/--no-print",
envvar="DBT_PRINT",
Expand Down Expand Up @@ -359,10 +369,6 @@
"--skip-profile-setup", "-s", envvar=None, help="Skip interactive profile setup.", is_flag=True
)

# TODO: The env var and name (reflected in flags) are corrections!
# The original name was `ARTIFACT_STATE_PATH` and used the env var `DBT_ARTIFACT_STATE_PATH`.
# Both of which break existing naming conventions.
# This will need to be fixed before use in the main codebase and communicated as a change to the community!
state = click.option(
"--state",
envvar="DBT_STATE",
Expand All @@ -376,6 +382,20 @@
),
)

deprecated_state = click.option(
"--deprecated-state",
envvar="DBT_ARTIFACT_STATE_PATH",
help="Internal flag for deprecating old env var.",
hidden=True,
type=click.Path(
dir_okay=True,
file_okay=False,
readable=True,
resolve_path=True,
path_type=Path,
),
)

static_parser = click.option(
"--static-parser/--no-static-parser",
envvar="DBT_STATIC_PARSER",
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/cli/requires.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def wrapper(*args, **kwargs):
fire_event(MainReportArgs(args=flags_dict_str))

# Deprecation warnings
[dep_fn() for dep_fn in flags.deprecated_env_var_warnings]
flags.fire_deprecations()

if active_user is not None: # mypy appeasement, always true
fire_event(MainTrackingUserState(user_state=active_user.state()))
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def code(self):
def message(self):
description = (
f"The environment variable `{self.old_name}` has been renamed as `{self.new_name}`.\n"
f"If `{self.old_name}` is currently set, its value will be used instead of `{self.old_name}`.\n"
f"If `{self.old_name}` is currently set, its value will be used instead of `{self.new_name}`.\n"
f"Set `{self.new_name}` and unset `{self.old_name}` to avoid this deprecation warning and "
"ensure it works properly in a future release."
)
Expand Down
60 changes: 60 additions & 0 deletions tests/functional/cli/test_env_var_deprecations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import pytest
import os

from dbt.tests.util import read_file, run_dbt


model_one_sql = """
select 1 as fun
"""


class TestDeprecatedEnvVars:
@pytest.fixture(scope="class")
def models(self):
return {"model_one.sql": model_one_sql}

def test_defer(self, project, logs_dir):
self.assert_deprecated(
logs_dir,
"DBT_DEFER_TO_STATE",
"DBT_DEFER",
)

def test_favor_state(self, project, logs_dir):
self.assert_deprecated(
logs_dir,
"DBT_FAVOR_STATE_MODE",
"DBT_FAVOR_STATE",
command="build",
)

def test_print(self, project, logs_dir):
self.assert_deprecated(
logs_dir,
"DBT_NO_PRINT",
"DBT_PRINT",
)

def test_state(self, project, logs_dir):
self.assert_deprecated(
logs_dir,
"DBT_ARTIFACT_STATE_PATH",
"DBT_STATE",
old_val=".",
)

def assert_deprecated(self, logs_dir, old_env_var, new_env_var, command="run", old_val="0"):
os.environ[old_env_var] = old_val
run_dbt([command])

# replacing new lines with spaces accounts for text wrapping
log_file = read_file(logs_dir, "dbt.log").replace("\n", " ").replace("\\n", " ")
dep_str = f"The environment variable `{old_env_var}` has been renamed as `{new_env_var}`"

try:
assert dep_str in log_file
except Exception as e:
del os.environ[old_env_var]
raise e
del os.environ[old_env_var]
Comment on lines +47 to +60
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not so sure about how I handle setting/unsetting env vars here. I am welcome to other suggestions.