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 c509d75 commit c678cbc
Show file tree
Hide file tree
Showing 6 changed files with 605 additions and 583 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)

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L433

Added line #L433 was not covered by tests
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#L435

Added line #L435 was not covered by tests

return line_wrap_message(description)


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

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."

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

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

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

View check run for this annotation

Codecov / codecov/patch

core/dbt/events/types.py#L451

Added line #L451 was not covered by tests
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#L453

Added line #L453 was not covered by tests

return line_wrap_message(description)


# =======================================================
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
6 changes: 4 additions & 2 deletions tests/unit/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,10 @@ def test_event_codes(self):
adapter_types.CollectFreshnessReturnSignature(),
core_types.TestsConfigDeprecation(deprecated_path="", exp_path=""),
core_types.ProjectFlagsMovedDeprecation(),
core_types.SpacesInModelNameDeprecation(model_name="", model_version=""),
core_types.TotalModelNamesWithSpacesDeprecation(count_invalid_names=1, show_debug_hint=True),
core_types.SpacesInModelNameDeprecation(model_name="", model_version="", level=""),
core_types.TotalModelNamesWithSpacesDeprecation(
count_invalid_names=1, show_debug_hint=True, level=""
),
# E - DB Adapter ======================
adapter_types.AdapterEventDebug(),
adapter_types.AdapterEventInfo(),
Expand Down

0 comments on commit c678cbc

Please sign in to comment.