Skip to content

Commit

Permalink
Add deprecation warnings for log-path, target-path in dbt_project…
Browse files Browse the repository at this point in the history
….yml (#7185)

* Add deprecation warnings for log-path, target-path in dbt_project.yml

* Fix tests/unit/test_events

* Fix failing tests

* PR feedback
  • Loading branch information
jtcohen6 authored Mar 21, 2023
1 parent 9605b76 commit 8225a00
Show file tree
Hide file tree
Showing 14 changed files with 151 additions and 29 deletions.
9 changes: 9 additions & 0 deletions .changes/unreleased/Breaking Changes-20230317-110033.yaml
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 2 additions & 0 deletions core/dbt/cli/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,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)
Expand Down
26 changes: 16 additions & 10 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,23 +298,27 @@ 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, 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)
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_value or project_dict[deprecated_path] != default_value:
deprecations.warn(
f"project-config-{deprecated_path}",
deprecated_path=deprecated_path,
)
deprecations.warn(
f"project-config-{deprecated_path}",
deprecated_path=deprecated_path,
exp_path=exp_path,
)

def create_project(self, rendered: RenderComponents) -> "Project":
unrendered = RenderComponents(
Expand All @@ -329,6 +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_value="logs")
self.check_config_path(rendered.project_dict, "target-path", default_value="target")

try:
ProjectContract.validate(rendered.project_dict)
Expand Down
12 changes: 12 additions & 0 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -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}
Expand Down
6 changes: 5 additions & 1 deletion core/dbt/events/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,8 @@ logger = AdapterLogger("<database name>")

## 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
```
26 changes: 26 additions & 0 deletions core/dbt/events/proto_types.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 25 additions & 5 deletions core/dbt/events/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ message ConfigDataPathDeprecationMsg {
ConfigDataPathDeprecation data = 2;
}

//D005
// D005
message AdapterDeprecationWarning {
string old_name = 1;
string new_name = 2;
Expand All @@ -316,7 +316,7 @@ message AdapterDeprecationWarningMsg {
AdapterDeprecationWarning data = 2;
}

//D006
// D006
message MetricAttributesRenamed {
string metric_name = 1;
}
Expand All @@ -326,7 +326,7 @@ message MetricAttributesRenamedMsg {
MetricAttributesRenamed data = 2;
}

//D007
// D007
message ExposureNameDeprecation {
string exposure = 1;
}
Expand All @@ -336,7 +336,7 @@ message ExposureNameDeprecationMsg {
ExposureNameDeprecation data = 2;
}

//D008
// D008
message InternalDeprecation {
string name = 1;
string reason = 2;
Expand All @@ -349,7 +349,7 @@ message InternalDeprecationMsg {
InternalDeprecation data = 2;
}

//D009
// D009
message EnvironmentVariableRenamed {
string old_name = 1;
string new_name = 2;
Expand All @@ -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
Expand Down
36 changes: 36 additions & 0 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
# =======================================================
Expand Down
1 change: 0 additions & 1 deletion core/dbt/include/starter_project/dbt_project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
11 changes: 5 additions & 6 deletions core/dbt/tests/fixtures/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -379,7 +381,6 @@ def __init__(
test_data_dir,
test_schema,
database,
logs_dir,
test_config,
):
self.project_root = project_root
Expand All @@ -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 = []

Expand Down Expand Up @@ -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()
Expand Down
13 changes: 11 additions & 2 deletions tests/functional/deprecations/test_deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 0 additions & 2 deletions tests/functional/init/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/partial_parsing/test_pp_vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit 8225a00

Please sign in to comment.