Skip to content

General Deprecation Warning Improvements #11465

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

Closed
Closed
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
61 changes: 59 additions & 2 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import abc
from dataclasses import dataclass
from typing import Callable, ClassVar, Dict, List, Optional, Set

import dbt.tracking
from dbt.events import types as core_types
from dbt_common.events.functions import warn_or_error
from dbt_common.events.base_types import EventLevel
from dbt_common.events.functions import fire_event, warn_or_error


class DBTDeprecation:
_name: ClassVar[Optional[str]] = None
_event: ClassVar[Optional[str]] = None
_summary_event: ClassVar[Optional[str]] = None

@property
def name(self) -> str:
Expand All @@ -33,6 +36,21 @@ def event(self) -> abc.ABCMeta:
raise NameError(msg)
raise NotImplementedError("event not implemented for {}".format(self._event))

@property
def summary_event(self) -> abc.ABCMeta:
if self._summary_event is not None:
module_path = core_types
class_name = self._summary_event
try:
return getattr(module_path, class_name)
except AttributeError:
msg = f"Event Class `{class_name}` is not defined in `{module_path}`"
raise NameError(msg)
else:
raise NotImplementedError(
"summary_event not implemented for {}".format(self._summary_event)
)

def show(self, *args, **kwargs) -> None:
if self.name not in active_deprecations:
event = self.event(**kwargs)
Expand Down Expand Up @@ -110,7 +128,8 @@ class PackageMaterializationOverrideDeprecation(DBTDeprecation):

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


class SourceFreshnessProjectHooksNotRun(DBTDeprecation):
Expand Down Expand Up @@ -198,3 +217,41 @@ def reset_deprecations():
def fire_buffered_deprecations():
[dep_fn() for dep_fn in buffered_deprecations]
buffered_deprecations.clear()


@dataclass
class ManagedDeprecationWarning:
deprecation_warning: DBTDeprecation
occurances: int = 0
only_show_once: bool = True

def show(self, fire_as_error: bool = False, *args, **kwargs) -> None:
# We don't use the `show` built into the DBTDeprecation class,
# because we want to handle gating whether the warning should is
# fired or not, and we want to track the number of times the warning
# has been shown.
if not self.only_show_once or self.occurances == 0:
event = self.deprecation_warning.event(*args, **kwargs)

# This is for maintaining current functionality wherein certain deprecations
# allow for force firing as an error outside of warn_or_error.
if fire_as_error:
fire_event(event, level=EventLevel.ERROR)
else:
warn_or_error(event)

self.deprecation_warning.track_deprecation_warn()

self.occurances += 1

def show_summary(self) -> None:
event = self.deprecation_warning.summary_event(
occurances=self.occurances,
show_debug_hint=(self.only_show_once),
)
warn_or_error(event)

if dbt.tracking.active_user is not None:
dbt.tracking.track_deprecation_warn_summary(
{"deprecation_name": self.deprecation_warning.name, "occurances": self.occurances}
)
3 changes: 2 additions & 1 deletion core/dbt/events/core_types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -417,9 +417,10 @@ message SpacesInResourceNameDeprecationMsg {

// D015
message ResourceNamesWithSpacesDeprecation {
int32 count_invalid_names = 1;
int32 count_invalid_names = 1; // Field no longer used, but kept for backwards compatibility
bool show_debug_hint = 2;
string level = 3;
int32 occurances = 4;
}

message ResourceNamesWithSpacesDeprecationMsg {
Expand Down
1,308 changes: 659 additions & 649 deletions core/dbt/events/core_types_pb2.py

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,9 @@ def code(self) -> str:
return "D015"

def message(self) -> str:
description = f"Spaces found in {self.count_invalid_names} resource name(s). This is deprecated, and may lead to errors when using dbt."
# set the count_invalid_names field to the occurances field for
self.count_invalid_names = self.occurances
description = f"Spaces found in {self.occurances} resource name(s). This is deprecated, and may lead to errors when using dbt."

if self.show_debug_hint:
description += " Run again with `--debug` to see them all."
Expand Down
34 changes: 16 additions & 18 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@
SourceDefinition,
)
from dbt.contracts.graph.semantic_manifest import SemanticManifest
from dbt.deprecations import (
ManagedDeprecationWarning,
ResourceNamesWithSpacesDeprecation,
)
from dbt.events.types import (
ArtifactWritten,
DeprecatedModel,
Expand All @@ -78,7 +82,6 @@
PartialParsingErrorProcessingFile,
PartialParsingNotEnabled,
PartialParsingSkipParsing,
SpacesInResourceNameDeprecation,
StateCheckVarsHash,
UnableToPartialParse,
UpcomingReferenceDeprecation,
Expand Down Expand Up @@ -618,42 +621,37 @@ def check_for_model_deprecations(self):
)
)

def check_for_spaces_in_resource_names(self):
def check_for_spaces_in_resource_names(self) -> None:
"""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 `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_resource_names = 0
level = (
EventLevel.ERROR
if self.root_project.args.REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES
else EventLevel.WARN
)

flags = get_flags()
deprecation_manager = ManagedDeprecationWarning(
deprecation_warning=ResourceNamesWithSpacesDeprecation(),
only_show_once=(not flags.DEBUG),
)

for node in self.manifest.nodes.values():
if " " in node.name:
if improper_resource_names == 0 or flags.DEBUG:
fire_event(
SpacesInResourceNameDeprecation(
unique_id=node.unique_id,
level=level.value,
),
level=level,
)
improper_resource_names += 1
deprecation_manager.show(
fire_as_error=(level == EventLevel.ERROR),
unique_id=node.unique_id,
level=level.value,
)

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

Expand Down
19 changes: 19 additions & 0 deletions core/dbt/tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

ADAPTER_INFO_SPEC = "iglu:com.dbt/adapter_info/jsonschema/1-0-1"
DEPRECATION_WARN_SPEC = "iglu:com.dbt/deprecation_warn/jsonschema/1-0-0"
DEPRECATION_WARN_SUMMARY_SPEC = "iglu:com.dbt/deprecation_warn_summary/jsonschema/1-0-0"
BEHAVIOR_CHANGE_WARN_SPEC = "iglu:com.dbt/behavior_change_warn/jsonschema/1-0-0"
EXPERIMENTAL_PARSER = "iglu:com.dbt/experimental_parser/jsonschema/1-0-0"
INVOCATION_ENV_SPEC = "iglu:com.dbt/invocation_env/jsonschema/1-0-0"
Expand Down Expand Up @@ -368,6 +369,24 @@ def track_deprecation_warn(options):
)


def track_deprecation_warn_summary(options):

assert (
active_user is not None
), "Cannot track deprecation warnings summary when active user is None"

context = [SelfDescribingJson(DEPRECATION_WARN_SUMMARY_SPEC, options)]

track(
active_user,
category="dbt",
action="deprecation_summary",
label=get_invocation_id(),
property_="warn",
context=context,
)


def track_behavior_change_warn(msg: EventMsg) -> None:
if msg.info.name != "BehaviorChangeEvent" or active_user is None:
return
Expand Down
4 changes: 1 addition & 3 deletions tests/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,7 @@ def test_event_codes(self):
core_types.TestsConfigDeprecation(deprecated_path="", exp_path=""),
core_types.ProjectFlagsMovedDeprecation(),
core_types.SpacesInResourceNameDeprecation(unique_id="", level=""),
core_types.ResourceNamesWithSpacesDeprecation(
count_invalid_names=1, show_debug_hint=True, level=""
),
core_types.ResourceNamesWithSpacesDeprecation(occurances=1, show_debug_hint=True, level=""),
core_types.PackageMaterializationOverrideDeprecation(
package_name="my_package", materialization_name="view"
),
Expand Down
Loading