Skip to content

Commit

Permalink
Improve readability of logs related to problematic model names
Browse files Browse the repository at this point in the history
We want people running dbt to be able to at a glance see warnings/errors
with running their project. In this case we are focused specifically on
errors/warnings in regards to model names containing spaces. Previously
we were only ever emitting the `warning_tag` in the message even if the
event itself was being emitted at an `ERROR` level. We now properly have
`[ERROR]` or `[WARNING]` in the message depending on the level. Unfortunately
we couldn't just look what level the event was being fired at, because that
information doesn't exist on the event itself.

Additionally, we're using events that base off of `DynamicEvents` which
unfortunately hard coded to `DEBUG`. Changing this would involve still
having a `level` property on the definition in `core_types.proto` and
then having `DynamicEvent`s look to `self.level` in the `level_tag`
method. Then we could change how firing events works based on the an
event's `level_tag` return value. This all sounds like a bit of tech
debt suited for PR, possibly multiple, and thus is not being done here.
  • Loading branch information
QMalcolm committed Apr 12, 2024
1 parent c94b70c commit 9865f4a
Show file tree
Hide file tree
Showing 5 changed files with 601 additions and 581 deletions.
2 changes: 2 additions & 0 deletions core/dbt/events/core_types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ message ProjectFlagsMovedDeprecationMsg {
message SpacesInModelNameDeprecation {
string model_name = 1;
string model_version = 2;
string level = 3;
}

message SpacesInModelNameDeprecationMsg {
Expand All @@ -418,6 +419,7 @@ message SpacesInModelNameDeprecationMsg {
message TotalModelNamesWithSpacesDeprecation {
int32 count_invalid_names = 1;
bool show_debug_hint = 2;
string level = 3;
}

message TotalModelNamesWithSpacesDeprecationMsg {
Expand Down
1,152 changes: 576 additions & 576 deletions core/dbt/events/core_types_pb2.py

Large diffs are not rendered by default.

22 changes: 19 additions & 3 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
from dbt.events.base_types import WarnLevel, InfoLevel, DebugLevel, ErrorLevel, DynamicLevel


# TODO Move this to dbt_common.ui
def _error_tag(msg: str) -> str:
return f'[{red("ERROR")}]: {msg}'

Check warning on line 16 in core/dbt/events/types.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L16

Added line #L16 was not covered by tests


# Event codes have prefixes which follow this table
#
# | Code | Description |
Expand Down Expand Up @@ -423,20 +428,31 @@ def message(self) -> str:
f"Model `{self.model_name}{version}` has spaces in its name. This is deprecated and "
"may cause errors when using dbt."
)
return line_wrap_message(warning_tag(f"Deprecated functionality\n\n{description}"))

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

Check warning on line 435 in core/dbt/events/types.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L432-L435

Added lines #L432 - L435 were not covered by tests

return line_wrap_message(description)

Check warning on line 437 in core/dbt/events/types.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L437

Added line #L437 was not covered by tests


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

Check warning on line 442 in core/dbt/events/types.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L442

Added line #L442 was not covered by tests

def message(self) -> str:
description = f"Found {self.count_invalid_names} models with spaces in their names, which is deprecated. "
description = f"Found {self.count_invalid_names} models with spaces in their names, which is deprecated."

Check warning on line 445 in core/dbt/events/types.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L445

Added line #L445 was not covered by tests

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

Check warning on line 448 in core/dbt/events/types.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L447-L448

Added lines #L447 - L448 were not covered by tests

return line_wrap_message(warning_tag(f"Deprecated functionality\n\n{description}"))
if self.level == EventLevel.ERROR.value:
description = _error_tag(description)
elif self.level == EventLevel.WARN.value:
description = warning_tag(description)

Check warning on line 453 in core/dbt/events/types.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L450-L453

Added lines #L450 - L453 were not covered by tests

return line_wrap_message(description)

Check warning on line 455 in core/dbt/events/types.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L455

Added line #L455 was not covered by tests


# =======================================================
Expand Down
2 changes: 2 additions & 0 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ def check_for_spaces_in_model_names(self):
SpacesInModelNameDeprecation(
model_name=node.name,
model_version=version_to_str(node.version),
level=level.value,
),
level=level,
)
Expand All @@ -657,6 +658,7 @@ def check_for_spaces_in_model_names(self):
TotalModelNamesWithSpacesDeprecation(
count_invalid_names=improper_model_names,
show_debug_hint=(not self.root_project.args.DEBUG),
level=level.value,
),
level=level,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def tests_debug_when_spaces_in_name(self, project) -> None:
"Found 2 models with spaces in their names" in total_catcher.caught_events[0].info.msg
)
assert (
"Run again with\n`--debug` to see them all." in total_catcher.caught_events[0].info.msg
"Run again with `--debug` to see them all." in total_catcher.caught_events[0].info.msg
)

spaces_check_catcher = EventCatcher(SpacesInModelNameDeprecation)
Expand All @@ -75,7 +75,7 @@ def tests_debug_when_spaces_in_name(self, project) -> None:
assert len(spaces_check_catcher.caught_events) == 2
assert len(total_catcher.caught_events) == 1
assert (
"Run again with\n`--debug` to see them all."
"Run again with `--debug` to see them all."
not in total_catcher.caught_events[0].info.msg
)

Expand Down

0 comments on commit 9865f4a

Please sign in to comment.