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 18fb56c
Show file tree
Hide file tree
Showing 5 changed files with 597 additions and 582 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.

19 changes: 15 additions & 4 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import json

from dbt.constants import MAXIMUM_SEED_SIZE_NAME, PIN_PACKAGE_URL
from dbt_common.ui import warning_tag, line_wrap_message, green, yellow, red
from dbt_common.ui import error_tag, warning_tag, line_wrap_message, green, yellow, red
from dbt_common.events.base_types import EventLevel
from dbt_common.events.format import (
format_fancy_output_line,
Expand Down Expand Up @@ -423,20 +423,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)

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)
elif self.level == EventLevel.WARN.value:
description = warning_tag(description)

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

0 comments on commit 18fb56c

Please sign in to comment.