From 289ff18d2e5cbcb16f7ddd74f05c4236cb592772 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Fri, 17 Mar 2023 10:44:45 +0100 Subject: [PATCH 1/4] Add deprecation warnings for log-path, target-path in dbt_project.yml --- .../Breaking Changes-20230317-110033.yaml | 9 +++++ core/dbt/cli/flags.py | 2 ++ core/dbt/config/project.py | 23 ++++++++---- core/dbt/deprecations.py | 12 +++++++ core/dbt/events/README.md | 6 +++- core/dbt/events/proto_types.py | 26 ++++++++++++++ core/dbt/events/types.proto | 30 +++++++++++++--- core/dbt/events/types.py | 36 +++++++++++++++++++ .../include/starter_project/dbt_project.yml | 1 - core/dbt/tests/fixtures/project.py | 11 +++--- .../deprecations/test_deprecations.py | 13 +++++-- 11 files changed, 148 insertions(+), 21 deletions(-) create mode 100644 .changes/unreleased/Breaking Changes-20230317-110033.yaml diff --git a/.changes/unreleased/Breaking Changes-20230317-110033.yaml b/.changes/unreleased/Breaking Changes-20230317-110033.yaml new file mode 100644 index 00000000000..ece17d8b1de --- /dev/null +++ b/.changes/unreleased/Breaking Changes-20230317-110033.yaml @@ -0,0 +1,9 @@ +kind: Breaking Changes +body: Specifying "log-path" and "target-path" in "dbt_project.yml" is deprecated. + This functionality will be removed in a future version of dbt-core. If you need + to specify a custom path for logs or artifacts, please set via CLI flag or env var + instead. +time: 2023-03-17T11:00:33.448472+01:00 +custom: + Author: jtcohen6 + Issue: "6882" diff --git a/core/dbt/cli/flags.py b/core/dbt/cli/flags.py index c0cae646688..84ade4c9ab9 100644 --- a/core/dbt/cli/flags.py +++ b/core/dbt/cli/flags.py @@ -213,6 +213,8 @@ def assign_params(ctx, params_assigned_from_default, deprecated_env_vars): self._override_if_set("LOG_FORMAT", "LOG_FORMAT_FILE", params_assigned_from_default) # Default LOG_PATH from PROJECT_DIR, if available. + # Starting in v1.5, if `log-path` is set in `dbt_project.yml`, it will raise a deprecation warning, + # with the possibility of removing it in a future release. if getattr(self, "LOG_PATH", None) is None: project_dir = getattr(self, "PROJECT_DIR", default_project_dir()) version_check = getattr(self, "VERSION_CHECK", True) diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index f0d3d94e3f6..78269b45dd8 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -298,7 +298,7 @@ def render_package_metadata(self, renderer: PackageRenderer) -> ProjectPackageMe raise DbtProjectError("Package dbt_project.yml must have a name!") return ProjectPackageMetadata(self.project_name, packages_config.packages) - def check_config_path(self, project_dict, deprecated_path, exp_path): + def check_config_path(self, project_dict, deprecated_path, exp_path=None, default=None): if deprecated_path in project_dict: if exp_path in project_dict: msg = ( @@ -310,11 +310,20 @@ def check_config_path(self, project_dict, deprecated_path, exp_path): raise DbtProjectError( msg.format(deprecated_path=deprecated_path, exp_path=exp_path) ) - deprecations.warn( - f"project-config-{deprecated_path}", - deprecated_path=deprecated_path, - exp_path=exp_path, - ) + # this field has been renamed + if exp_path: + deprecations.warn( + f"project-config-{deprecated_path}", + deprecated_path=deprecated_path, + exp_path=exp_path, + ) + # this field is no longer supported, but many projects may specify it with the default value + # if so, let's only raise this deprecation warning if they set a custom value + if not default or project_dict[deprecated_path] != default: + deprecations.warn( + f"project-config-{deprecated_path}", + deprecated_path=deprecated_path, + ) def create_project(self, rendered: RenderComponents) -> "Project": unrendered = RenderComponents( @@ -329,6 +338,8 @@ def create_project(self, rendered: RenderComponents) -> "Project": self.check_config_path(rendered.project_dict, "source-paths", "model-paths") self.check_config_path(rendered.project_dict, "data-paths", "seed-paths") + self.check_config_path(rendered.project_dict, "log-path", default="logs") + self.check_config_path(rendered.project_dict, "target-path", default="target") try: ProjectContract.validate(rendered.project_dict) diff --git a/core/dbt/deprecations.py b/core/dbt/deprecations.py index 51545f28079..3f91866bceb 100644 --- a/core/dbt/deprecations.py +++ b/core/dbt/deprecations.py @@ -81,6 +81,16 @@ class ExposureNameDeprecation(DBTDeprecation): _event = "ExposureNameDeprecation" +class ConfigLogPathDeprecation(DBTDeprecation): + _name = "project-config-log-path" + _event = "ConfigLogPathDeprecation" + + +class ConfigTargetPathDeprecation(DBTDeprecation): + _name = "project-config-target-path" + _event = "ConfigTargetPathDeprecation" + + def renamed_env_var(old_name: str, new_name: str): class EnvironmentVariableRenamed(DBTDeprecation): _name = f"environment-variable-renamed:{old_name}" @@ -116,6 +126,8 @@ def warn(name, *args, **kwargs): ConfigDataPathDeprecation(), MetricAttributesRenamed(), ExposureNameDeprecation(), + ConfigLogPathDeprecation(), + ConfigTargetPathDeprecation(), ] deprecations: Dict[str, DBTDeprecation] = {d.name: d for d in deprecations_list} diff --git a/core/dbt/events/README.md b/core/dbt/events/README.md index 52edd7d35d4..c4674ccf40d 100644 --- a/core/dbt/events/README.md +++ b/core/dbt/events/README.md @@ -50,4 +50,8 @@ logger = AdapterLogger("") ## Compiling types.proto -After adding a new message in types.proto, in the core/dbt/events directory: ```protoc --python_betterproto_out . types.proto``` +After adding a new message in types.proto: +``` +cd core/dbt/events +protoc --python_betterproto_out . types.proto +``` diff --git a/core/dbt/events/proto_types.py b/core/dbt/events/proto_types.py index 24d70ec9947..acb968c2654 100644 --- a/core/dbt/events/proto_types.py +++ b/core/dbt/events/proto_types.py @@ -460,6 +460,32 @@ class EnvironmentVariableRenamedMsg(betterproto.Message): data: "EnvironmentVariableRenamed" = betterproto.message_field(2) +@dataclass +class ConfigLogPathDeprecation(betterproto.Message): + """D010""" + + deprecated_path: str = betterproto.string_field(1) + + +@dataclass +class ConfigLogPathDeprecationMsg(betterproto.Message): + info: "EventInfo" = betterproto.message_field(1) + data: "ConfigLogPathDeprecation" = betterproto.message_field(2) + + +@dataclass +class ConfigTargetPathDeprecation(betterproto.Message): + """D011""" + + deprecated_path: str = betterproto.string_field(1) + + +@dataclass +class ConfigTargetPathDeprecationMsg(betterproto.Message): + info: "EventInfo" = betterproto.message_field(1) + data: "ConfigTargetPathDeprecation" = betterproto.message_field(2) + + @dataclass class AdapterEventDebug(betterproto.Message): """E001""" diff --git a/core/dbt/events/types.proto b/core/dbt/events/types.proto index 39a53257e69..f7ced23f1d5 100644 --- a/core/dbt/events/types.proto +++ b/core/dbt/events/types.proto @@ -305,7 +305,7 @@ message ConfigDataPathDeprecationMsg { ConfigDataPathDeprecation data = 2; } -//D005 +// D005 message AdapterDeprecationWarning { string old_name = 1; string new_name = 2; @@ -316,7 +316,7 @@ message AdapterDeprecationWarningMsg { AdapterDeprecationWarning data = 2; } -//D006 +// D006 message MetricAttributesRenamed { string metric_name = 1; } @@ -326,7 +326,7 @@ message MetricAttributesRenamedMsg { MetricAttributesRenamed data = 2; } -//D007 +// D007 message ExposureNameDeprecation { string exposure = 1; } @@ -336,7 +336,7 @@ message ExposureNameDeprecationMsg { ExposureNameDeprecation data = 2; } -//D008 +// D008 message InternalDeprecation { string name = 1; string reason = 2; @@ -349,7 +349,7 @@ message InternalDeprecationMsg { InternalDeprecation data = 2; } -//D009 +// D009 message EnvironmentVariableRenamed { string old_name = 1; string new_name = 2; @@ -360,6 +360,26 @@ message EnvironmentVariableRenamedMsg { EnvironmentVariableRenamed data = 2; } +// D010 +message ConfigLogPathDeprecation { + string deprecated_path = 1; +} + +message ConfigLogPathDeprecationMsg { + EventInfo info = 1; + ConfigLogPathDeprecation data = 2; +} + +// D011 +message ConfigTargetPathDeprecation { + string deprecated_path = 1; +} + +message ConfigTargetPathDeprecationMsg { + EventInfo info = 1; + ConfigTargetPathDeprecation data = 2; +} + // E - DB Adapter // E001 diff --git a/core/dbt/events/types.py b/core/dbt/events/types.py index 9ff34f84b52..c652ede4f4f 100644 --- a/core/dbt/events/types.py +++ b/core/dbt/events/types.py @@ -408,6 +408,42 @@ def message(self): return line_wrap_message(warning_tag(f"Deprecated functionality\n\n{description}")) +@dataclass +class ConfigLogPathDeprecation(WarnLevel, pt.ConfigSourcePathDeprecation): # noqa + def code(self): + return "D010" + + def message(self): + output = "logs" + cli_flag = "--log-path" + env_var = "DBT_LOG_PATH" + description = ( + f"The `{self.deprecated_path}` config in `dbt_project.yml` has been deprecated, " + f"and will no longer be supported in a future version of dbt-core. " + f"If you wish to write dbt {output} to a custom directory, please use " + f"the {cli_flag} CLI flag or {env_var} env var instead." + ) + return line_wrap_message(warning_tag(f"Deprecated functionality\n\n{description}")) + + +@dataclass +class ConfigTargetPathDeprecation(WarnLevel, pt.ConfigSourcePathDeprecation): # noqa + def code(self): + return "D011" + + def message(self): + output = "artifacts" + cli_flag = "--target-path" + env_var = "DBT_TARGET_PATH" + description = ( + f"The `{self.deprecated_path}` config in `dbt_project.yml` has been deprecated, " + f"and will no longer be supported in a future version of dbt-core. " + f"If you wish to write dbt {output} to a custom directory, please use " + f"the {cli_flag} CLI flag or {env_var} env var instead." + ) + return line_wrap_message(warning_tag(f"Deprecated functionality\n\n{description}")) + + # ======================================================= # E - DB Adapter # ======================================================= diff --git a/core/dbt/include/starter_project/dbt_project.yml b/core/dbt/include/starter_project/dbt_project.yml index 826a5600f4e..630001eed2f 100644 --- a/core/dbt/include/starter_project/dbt_project.yml +++ b/core/dbt/include/starter_project/dbt_project.yml @@ -19,7 +19,6 @@ seed-paths: ["seeds"] macro-paths: ["macros"] snapshot-paths: ["snapshots"] -target-path: "target" # directory which will store compiled SQL files clean-targets: # directories to be removed by `dbt clean` - "target" - "dbt_packages" diff --git a/core/dbt/tests/fixtures/project.py b/core/dbt/tests/fixtures/project.py index bfdb4dfeab7..23243caef58 100644 --- a/core/dbt/tests/fixtures/project.py +++ b/core/dbt/tests/fixtures/project.py @@ -177,11 +177,10 @@ def project_config_update(): # Combines the project_config_update dictionary with project_config defaults to # produce a project_yml config and write it out as dbt_project.yml @pytest.fixture(scope="class") -def dbt_project_yml(project_root, project_config_update, logs_dir): +def dbt_project_yml(project_root, project_config_update): project_config = { "name": "test", "profile": "test", - "log-path": logs_dir, } if project_config_update: if isinstance(project_config_update, dict): @@ -355,7 +354,10 @@ def project_files(project_root, models, macros, snapshots, properties, seeds, te # We have a separate logs dir for every test @pytest.fixture(scope="class") def logs_dir(request, prefix): - return os.path.join(request.config.rootdir, "logs", prefix) + dbt_log_dir = os.path.join(request.config.rootdir, "logs", prefix) + os.environ["DBT_LOG_PATH"] = str(dbt_log_dir) + yield dbt_log_dir + del os.environ["DBT_LOG_PATH"] # This fixture is for customizing tests that need overrides in adapter @@ -379,7 +381,6 @@ def __init__( test_data_dir, test_schema, database, - logs_dir, test_config, ): self.project_root = project_root @@ -390,7 +391,6 @@ def __init__( self.test_data_dir = test_data_dir self.test_schema = test_schema self.database = database - self.logs_dir = logs_dir self.test_config = test_config self.created_schemas = [] @@ -498,7 +498,6 @@ def project( test_data_dir=test_data_dir, test_schema=unique_schema, database=adapter.config.credentials.database, - logs_dir=logs_dir, test_config=test_config, ) project.drop_test_schema() diff --git a/tests/functional/deprecations/test_deprecations.py b/tests/functional/deprecations/test_deprecations.py index a70b3687c69..4b71e0df0e7 100644 --- a/tests/functional/deprecations/test_deprecations.py +++ b/tests/functional/deprecations/test_deprecations.py @@ -51,13 +51,22 @@ def models(self): @pytest.fixture(scope="class") def project_config_update(self): - return {"config-version": 2, "data-paths": ["data"]} + return { + "config-version": 2, + "data-paths": ["data"], + "log-path": "customlogs", + "target-path": "customtarget", + } def test_data_path(self, project): deprecations.reset_deprecations() assert deprecations.active_deprecations == set() run_dbt(["debug"]) - expected = {"project-config-data-paths"} + expected = { + "project-config-data-paths", + "project-config-log-path", + "project-config-target-path", + } assert expected == deprecations.active_deprecations def test_data_path_fail(self, project): From 9e19a86098f335fd665de031338c808ab6b48aaa Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Fri, 17 Mar 2023 11:08:37 +0100 Subject: [PATCH 2/4] Fix tests/unit/test_events --- tests/unit/test_events.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_events.py b/tests/unit/test_events.py index 01e9e576cd2..2b4f8126eea 100644 --- a/tests/unit/test_events.py +++ b/tests/unit/test_events.py @@ -127,6 +127,8 @@ def test_event_codes(self): types.ExposureNameDeprecation(exposure=""), types.InternalDeprecation(name="", reason="", suggested_action="", version=""), types.EnvironmentVariableRenamed(old_name="", new_name=""), + types.ConfigLogPathDeprecation(deprecated_path=""), + types.ConfigTargetPathDeprecation(deprecated_path=""), # E - DB Adapter ====================== types.AdapterEventDebug(), types.AdapterEventInfo(), From e32b0eccb7d2a0502247fb8df35f27881ce2ee6e Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Fri, 17 Mar 2023 11:46:46 +0100 Subject: [PATCH 3/4] Fix failing tests --- tests/functional/init/test_init.py | 2 -- tests/functional/partial_parsing/test_pp_vars.py | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/functional/init/test_init.py b/tests/functional/init/test_init.py index 6a79dfe9807..4e48eeeffb5 100644 --- a/tests/functional/init/test_init.py +++ b/tests/functional/init/test_init.py @@ -547,7 +547,6 @@ def test_init_task_outside_of_project( macro-paths: ["macros"] snapshot-paths: ["snapshots"] -target-path: "target" # directory which will store compiled SQL files clean-targets: # directories to be removed by `dbt clean` - "target" - "dbt_packages" @@ -667,7 +666,6 @@ def test_init_provided_project_name_and_skip_profile_setup( macro-paths: ["macros"] snapshot-paths: ["snapshots"] -target-path: "target" # directory which will store compiled SQL files clean-targets: # directories to be removed by `dbt clean` - "target" - "dbt_packages" diff --git a/tests/functional/partial_parsing/test_pp_vars.py b/tests/functional/partial_parsing/test_pp_vars.py index c306ef075db..a73d250eb30 100644 --- a/tests/functional/partial_parsing/test_pp_vars.py +++ b/tests/functional/partial_parsing/test_pp_vars.py @@ -317,7 +317,7 @@ def dbt_profile_target(self): "dbname": "dbt", } - def test_profile_env_vars(self, project): + def test_profile_env_vars(self, project, logs_dir): # Initial run os.environ["ENV_VAR_USER"] = "root" @@ -334,7 +334,7 @@ def test_profile_env_vars(self, project): with pytest.raises(FailedToConnectError): run_dbt(["run"], expect_pass=False) - log_output = Path(project.logs_dir, "dbt.log").read_text() + log_output = Path(logs_dir, "dbt.log").read_text() assert "env vars used in profiles.yml have changed" in log_output manifest = get_manifest(project.project_root) From 93585ca10a96de8f3027aa52922087beb00f0af8 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Tue, 21 Mar 2023 20:32:13 +0100 Subject: [PATCH 4/4] PR feedback --- core/dbt/config/project.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 78269b45dd8..76a3980e26a 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -298,28 +298,23 @@ def render_package_metadata(self, renderer: PackageRenderer) -> ProjectPackageMe raise DbtProjectError("Package dbt_project.yml must have a name!") return ProjectPackageMetadata(self.project_name, packages_config.packages) - def check_config_path(self, project_dict, deprecated_path, exp_path=None, default=None): + def check_config_path( + self, project_dict, deprecated_path, expected_path=None, default_value=None + ): if deprecated_path in project_dict: - if exp_path in project_dict: + if expected_path in project_dict: msg = ( - "{deprecated_path} and {exp_path} cannot both be defined. The " - "`{deprecated_path}` config has been deprecated in favor of `{exp_path}`. " + "{deprecated_path} and {expected_path} cannot both be defined. The " + "`{deprecated_path}` config has been deprecated in favor of `{expected_path}`. " "Please update your `dbt_project.yml` configuration to reflect this " "change." ) raise DbtProjectError( - msg.format(deprecated_path=deprecated_path, exp_path=exp_path) - ) - # this field has been renamed - if exp_path: - deprecations.warn( - f"project-config-{deprecated_path}", - deprecated_path=deprecated_path, - exp_path=exp_path, + msg.format(deprecated_path=deprecated_path, expected_path=expected_path) ) # this field is no longer supported, but many projects may specify it with the default value # if so, let's only raise this deprecation warning if they set a custom value - if not default or project_dict[deprecated_path] != default: + if not default_value or project_dict[deprecated_path] != default_value: deprecations.warn( f"project-config-{deprecated_path}", deprecated_path=deprecated_path, @@ -338,8 +333,8 @@ def create_project(self, rendered: RenderComponents) -> "Project": self.check_config_path(rendered.project_dict, "source-paths", "model-paths") self.check_config_path(rendered.project_dict, "data-paths", "seed-paths") - self.check_config_path(rendered.project_dict, "log-path", default="logs") - self.check_config_path(rendered.project_dict, "target-path", default="target") + self.check_config_path(rendered.project_dict, "log-path", default_value="logs") + self.check_config_path(rendered.project_dict, "target-path", default_value="target") try: ProjectContract.validate(rendered.project_dict)