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

Consistent behavior change flag names + deprecation warnings #10063

Merged
merged 12 commits into from
May 1, 2024
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20240429-142342.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Consistent naming + deprecation warnings for "legacy behavior" flags
time: 2024-04-29T14:23:42.804244+02:00
custom:
Author: jtcohen6
Issue: "10062"
12 changes: 7 additions & 5 deletions core/dbt/contracts/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ def validate(cls, data):

@dataclass
class ProjectFlags(ExtensibleDbtClassMixin):
allow_spaces_in_model_names: Optional[bool] = True
cache_selected_only: Optional[bool] = None
debug: Optional[bool] = None
fail_fast: Optional[bool] = None
Expand All @@ -321,9 +320,7 @@ class ProjectFlags(ExtensibleDbtClassMixin):
partial_parse: Optional[bool] = None
populate_cache: Optional[bool] = None
printer_width: Optional[int] = None
require_explicit_package_overrides_for_builtin_materializations: bool = False
send_anonymous_usage_stats: bool = DEFAULT_SEND_ANONYMOUS_USAGE_STATS
source_freshness_run_project_hooks: bool = False
static_parser: Optional[bool] = None
use_colors: Optional[bool] = None
use_colors_file: Optional[bool] = None
Expand All @@ -333,12 +330,17 @@ class ProjectFlags(ExtensibleDbtClassMixin):
warn_error_options: Optional[Dict[str, Union[str, List[str]]]] = None
write_json: Optional[bool] = None

# legacy behaviors
require_explicit_package_overrides_for_builtin_materializations: bool = True
require_resource_names_without_spaces: bool = False
source_freshness_run_project_hooks: bool = False

@property
def project_only_flags(self) -> Dict[str, Any]:
return {
"source_freshness_run_project_hooks": self.source_freshness_run_project_hooks,
"allow_spaces_in_model_names": self.allow_spaces_in_model_names,
"require_explicit_package_overrides_for_builtin_materializations": self.require_explicit_package_overrides_for_builtin_materializations,
"require_resource_names_without_spaces": self.require_resource_names_without_spaces,
"source_freshness_run_project_hooks": self.source_freshness_run_project_hooks,
}


Expand Down
12 changes: 12 additions & 0 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,16 @@ class PackageMaterializationOverrideDeprecation(DBTDeprecation):
_event = "PackageMaterializationOverrideDeprecation"


class ResourceNamesWithSpacesDeprecation(DBTDeprecation):
_name = "resource-names-with-spaces"
_event = "ResourceNamesWithSpacesDeprecation"


class SourceFreshnessProjectHooksNotRun(DBTDeprecation):
_name = "source-freshness-project-hooks"
_event = "SourceFreshnessProjectHooksNotRun"


def renamed_env_var(old_name: str, new_name: str):
class EnvironmentVariableRenamed(DBTDeprecation):
_name = f"environment-variable-renamed:{old_name}"
Expand Down Expand Up @@ -163,6 +173,8 @@ def warn(name, *args, **kwargs):
TestsConfigDeprecation(),
ProjectFlagsMovedDeprecation(),
PackageMaterializationOverrideDeprecation(),
ResourceNamesWithSpacesDeprecation(),
SourceFreshnessProjectHooksNotRun(),
]

deprecations: Dict[str, DBTDeprecation] = {d.name: d for d in deprecations_list}
Expand Down
25 changes: 16 additions & 9 deletions core/dbt/events/core_types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -404,24 +404,28 @@ message ProjectFlagsMovedDeprecationMsg {
}

// D014
message SpacesInModelNameDeprecation {
string model_name = 1;
string model_version = 2;
string level = 3;
message SpacesInResourceNameDeprecation {
string unique_id = 1;
string level = 2;
}

message SpacesInModelNameDeprecationMsg {
message SpacesInResourceNameDeprecationMsg {
CoreEventInfo info = 1;
SpacesInModelNameDeprecation data = 2;
SpacesInResourceNameDeprecation data = 2;
}

// D015
message TotalModelNamesWithSpacesDeprecation {
message ResourceNamesWithSpacesDeprecation {
int32 count_invalid_names = 1;
bool show_debug_hint = 2;
string level = 3;
}

message ResourceNamesWithSpacesDeprecationMsg {
CoreEventInfo info = 1;
ResourceNamesWithSpacesDeprecation data = 2;
}

// D016
message PackageMaterializationOverrideDeprecation {
string package_name = 1;
Expand All @@ -433,9 +437,12 @@ message PackageMaterializationOverrideDeprecationMsg {
PackageMaterializationOverrideDeprecation data = 2;
}

message TotalModelNamesWithSpacesDeprecationMsg {
// D017
message SourceFreshnessProjectHooksNotRun {}

message SourceFreshnessProjectHooksNotRunMsg {
CoreEventInfo info = 1;
TotalModelNamesWithSpacesDeprecation data = 2;
SourceFreshnessProjectHooksNotRun data = 2;
}

// I065
Expand Down
1,166 changes: 585 additions & 581 deletions core/dbt/events/core_types_pb2.py

Large diffs are not rendered by default.

29 changes: 15 additions & 14 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,16 +413,12 @@ def message(self) -> str:
return warning_tag(f"Deprecated functionality\n\n{description}")


class SpacesInModelNameDeprecation(DynamicLevel):
class SpacesInResourceNameDeprecation(DynamicLevel):
def code(self) -> str:
return "D014"

def message(self) -> str:
version = ".v" + self.model_version if self.model_version else ""
description = (
f"Model `{self.model_name}{version}` has spaces in its name. This is deprecated and "
"may cause errors when using dbt."
)
description = f"Found spaces in the name of `{self.unique_id}`"

if self.level == EventLevel.ERROR.value:
description = error_tag(description)
Expand All @@ -432,22 +428,17 @@ def message(self) -> str:
return line_wrap_message(description)


class TotalModelNamesWithSpacesDeprecation(DynamicLevel):
class ResourceNamesWithSpacesDeprecation(WarnLevel):
def code(self) -> str:
return "D015"

def message(self) -> str:
description = f"Spaces in model names found in {self.count_invalid_names} model(s), which is deprecated."
description = f"Spaces found in {self.count_invalid_names} resource name(s). This is deprecated, and may lead to errors when using dbt. For more information: https://docs.getdbt.com/reference/global-configs/legacy-behaviors#require_resource_names_without_spaces"

if self.show_debug_hint:
description += " Run again with `--debug` to see them all."

if self.level == EventLevel.ERROR.value:
description = error_tag(description)
elif self.level == EventLevel.WARN.value:
description = warning_tag(description)

return line_wrap_message(description)
return line_wrap_message(warning_tag(description))


class PackageMaterializationOverrideDeprecation(WarnLevel):
Expand All @@ -460,6 +451,16 @@ def message(self) -> str:
return line_wrap_message(warning_tag(description))


class SourceFreshnessProjectHooksNotRun(WarnLevel):
def code(self) -> str:
return "D017"

def message(self) -> str:
description = "In a future version of dbt, the `source freshness` command will start running `on-run-start` and `on-run-end` hooks by default. Please refer to https://docs.getdbt.com/reference/global-configs/legacy-behaviors#source_freshness_run_project_hooks for detailed documentation and suggested workarounds."

return line_wrap_message(warning_tag(description))


# =======================================================
# I - Project parsing
# =======================================================
Expand Down
57 changes: 27 additions & 30 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from dbt.mp_context import get_mp_context
import msgpack

import dbt.deprecations
import dbt.exceptions
import dbt.tracking
import dbt.utils
Expand Down Expand Up @@ -64,9 +65,8 @@
StateCheckVarsHash,
DeprecatedModel,
DeprecatedReference,
SpacesInModelNameDeprecation,
TotalModelNamesWithSpacesDeprecation,
UpcomingReferenceDeprecation,
SpacesInResourceNameDeprecation,
)
from dbt.logger import DbtProcessState
from dbt.node_types import NodeType, AccessType
Expand Down Expand Up @@ -525,7 +525,7 @@ def load(self) -> Manifest:
self.write_manifest_for_partial_parse()

self.check_for_model_deprecations()
self.check_for_spaces_in_model_names()
self.check_for_spaces_in_resource_names()

return self.manifest

Expand Down Expand Up @@ -627,46 +627,43 @@ def check_for_model_deprecations(self):
)
)

def check_for_spaces_in_model_names(self):
"""Validates that model names do not contain spaces
def check_for_spaces_in_resource_names(self):
"""Validates that resource names do not contain spaces

If `DEBUG` flag is `False`, logs only first bad model name
If `DEBUG` flag is `True`, logs every bad model name
If `ALLOW_SPACES_IN_MODEL_NAMES` is `False`, logs are `ERROR` level and an exception is raised if any names are bad
If `ALLOW_SPACES_IN_MODEL_NAMES` is `True`, logs are `WARN` level
If `REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES` is `True`, logs are `ERROR` level and an exception is raised if any names are bad
If `REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES` is `False`, logs are `WARN` level
"""
improper_model_names = 0
improper_resource_names = 0
level = (
EventLevel.WARN
if self.root_project.args.ALLOW_SPACES_IN_MODEL_NAMES
else EventLevel.ERROR
EventLevel.ERROR
if self.root_project.args.REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES
else EventLevel.WARN
)

for node in self.manifest.nodes.values():
if isinstance(node, ModelNode) and " " in node.name:
if improper_model_names == 0 or self.root_project.args.DEBUG:
if " " in node.name:
if improper_resource_names == 0 or self.root_project.args.DEBUG:
fire_event(
SpacesInModelNameDeprecation(
model_name=node.name,
model_version=version_to_str(node.version),
SpacesInResourceNameDeprecation(
unique_id=node.unique_id,
level=level.value,
),
level=level,
)
improper_model_names += 1

if improper_model_names > 0:
fire_event(
TotalModelNamesWithSpacesDeprecation(
count_invalid_names=improper_model_names,
show_debug_hint=(not self.root_project.args.DEBUG),
level=level.value,
),
level=level,
)

if level == EventLevel.ERROR:
raise DbtValidationError("Model names cannot contain spaces")
improper_resource_names += 1

if improper_resource_names > 0:
if level == EventLevel.WARN:
flags = get_flags()
dbt.deprecations.warn(
"resource-names-with-spaces",
count_invalid_names=improper_resource_names,
show_debug_hint=(not flags.DEBUG),
)
else: # ERROR level
raise DbtValidationError("Resource names cannot contain spaces")

def load_and_parse_macros(self, project_parser_files):
for project in self.all_projects.values():
Expand Down
4 changes: 4 additions & 0 deletions core/dbt/task/freshness.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from dbt_common.exceptions import DbtRuntimeError, DbtInternalError
from dbt_common.events.functions import fire_event
from dbt_common.events.types import Note
from dbt import deprecations
from dbt.events.types import (
FreshnessCheckComplete,
LogStartLine,
Expand Down Expand Up @@ -240,9 +241,12 @@
fire_event(FreshnessCheckComplete())

def get_hooks_by_type(self, hook_type: RunHookType) -> List[HookNode]:
hooks = super().get_hooks_by_type(hook_type)
MichelleArk marked this conversation as resolved.
Show resolved Hide resolved
if self.args.source_freshness_run_project_hooks:
return super().get_hooks_by_type(hook_type)
else:
if hooks:
deprecations.warn("source-freshness-project-hooks")

Check warning on line 249 in core/dbt/task/freshness.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/task/freshness.py#L249

Added line #L249 was not covered by tests
return []

def populate_metadata_freshness_cache(self, adapter, selected_uids: AbstractSet[str]) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

from dataclasses import dataclass, field
from dbt.cli.main import dbtRunner
from dbt import deprecations
from dbt_common.events.base_types import BaseEvent, EventLevel, EventMsg
from dbt.events.types import SpacesInModelNameDeprecation, TotalModelNamesWithSpacesDeprecation
from dbt.events.types import SpacesInResourceNameDeprecation, ResourceNamesWithSpacesDeprecation
from dbt.tests.util import update_config_file
from typing import Dict, List

Expand All @@ -20,7 +21,7 @@ def catch(self, event: EventMsg):

class TestSpacesInModelNamesHappyPath:
def test_no_warnings_when_no_spaces_in_name(self, project) -> None:
event_catcher = EventCatcher(SpacesInModelNameDeprecation)
event_catcher = EventCatcher(SpacesInResourceNameDeprecation)
runner = dbtRunner(callbacks=[event_catcher.catch])
runner.invoke(["parse"])
assert len(event_catcher.caught_events) == 0
Expand All @@ -34,15 +35,15 @@ def models(self) -> Dict[str, str]:
}

def tests_warning_when_spaces_in_name(self, project) -> None:
event_catcher = EventCatcher(SpacesInModelNameDeprecation)
total_catcher = EventCatcher(TotalModelNamesWithSpacesDeprecation)
event_catcher = EventCatcher(SpacesInResourceNameDeprecation)
total_catcher = EventCatcher(ResourceNamesWithSpacesDeprecation)
runner = dbtRunner(callbacks=[event_catcher.catch, total_catcher.catch])
runner.invoke(["parse"])

assert len(total_catcher.caught_events) == 1
assert len(event_catcher.caught_events) == 1
event = event_catcher.caught_events[0]
assert "Model `my model` has spaces in its name. This is deprecated" in event.info.msg
assert "Found spaces in the name of `model.test.my model`" in event.info.msg
assert event.info.level == EventLevel.WARN


Expand All @@ -55,21 +56,21 @@ def models(self) -> Dict[str, str]:
}

def tests_debug_when_spaces_in_name(self, project) -> None:
spaces_check_catcher = EventCatcher(SpacesInModelNameDeprecation)
total_catcher = EventCatcher(TotalModelNamesWithSpacesDeprecation)
deprecations.reset_deprecations()
spaces_check_catcher = EventCatcher(SpacesInResourceNameDeprecation)
total_catcher = EventCatcher(ResourceNamesWithSpacesDeprecation)
runner = dbtRunner(callbacks=[spaces_check_catcher.catch, total_catcher.catch])
runner.invoke(["parse"])
assert len(spaces_check_catcher.caught_events) == 1
assert len(total_catcher.caught_events) == 1
assert (
"Spaces in model names found in 2 model(s)" in total_catcher.caught_events[0].info.msg
)
assert "Spaces found in 2 resource name(s)" in total_catcher.caught_events[0].info.msg
assert (
"Run again with `--debug` to see them all." in total_catcher.caught_events[0].info.msg
)

spaces_check_catcher = EventCatcher(SpacesInModelNameDeprecation)
total_catcher = EventCatcher(TotalModelNamesWithSpacesDeprecation)
deprecations.reset_deprecations()
spaces_check_catcher = EventCatcher(SpacesInResourceNameDeprecation)
total_catcher = EventCatcher(ResourceNamesWithSpacesDeprecation)
runner = dbtRunner(callbacks=[spaces_check_catcher.catch, total_catcher.catch])
runner.invoke(["parse", "--debug"])
assert len(spaces_check_catcher.caught_events) == 2
Expand All @@ -87,20 +88,20 @@ def models(self) -> Dict[str, str]:
"my model.sql": "select 1 as id",
}

def test_dont_allow_spaces_in_model_names(self, project):
spaces_check_catcher = EventCatcher(SpacesInModelNameDeprecation)
def test_require_resource_names_without_spaces(self, project):
spaces_check_catcher = EventCatcher(SpacesInResourceNameDeprecation)
runner = dbtRunner(callbacks=[spaces_check_catcher.catch])
runner.invoke(["parse"])
assert len(spaces_check_catcher.caught_events) == 1
assert spaces_check_catcher.caught_events[0].info.level == EventLevel.WARN

config_patch = {"flags": {"allow_spaces_in_model_names": False}}
config_patch = {"flags": {"require_resource_names_without_spaces": True}}
update_config_file(config_patch, project.project_root, "dbt_project.yml")

spaces_check_catcher = EventCatcher(SpacesInModelNameDeprecation)
spaces_check_catcher = EventCatcher(SpacesInResourceNameDeprecation)
runner = dbtRunner(callbacks=[spaces_check_catcher.catch])
result = runner.invoke(["parse"])
assert not result.success
assert "Model names cannot contain spaces" in result.exception.__str__()
assert "Resource names cannot contain spaces" in result.exception.__str__()
assert len(spaces_check_catcher.caught_events) == 1
assert spaces_check_catcher.caught_events[0].info.level == EventLevel.ERROR
Loading