Begin warning people about spaces in model names

.changes/unreleased/Fixes-20240409-233347.yaml

@@ -0,0 +1,6 @@
+kind: Fixes
+body: Begin warning people about spaces in model names
+time: 2024-04-09T23:33:47.850166-07:00
+custom:
+  Author: QMalcolm
+  Issue: "9397"

core/dbt/contracts/project.py

@@ -296,6 +296,7 @@ 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
@@ -320,7 +321,10 @@ class ProjectFlags(ExtensibleDbtClassMixin):
 
     @property
     def project_only_flags(self) -> Dict[str, Any]:
-        return {"source_freshness_run_project_hooks": self.source_freshness_run_project_hooks}
+        return {
+            "source_freshness_run_project_hooks": self.source_freshness_run_project_hooks,
+            "allow_spaces_in_model_names": self.allow_spaces_in_model_names,
+        }
 
 
 @dataclass

core/dbt/events/core_types.proto

@@ -403,6 +403,30 @@ message ProjectFlagsMovedDeprecationMsg {
     ProjectFlagsMovedDeprecation data = 2;
 }
 
+// D014
+message SpacesInModelNameDeprecation {
+    string model_name = 1;
+    string model_version = 2;
+    string level = 3;
+}
+
+message SpacesInModelNameDeprecationMsg {
+    CoreEventInfo info = 1;
+    SpacesInModelNameDeprecation data = 2;
+}
+
+// D015
+message TotalModelNamesWithSpacesDeprecation {
+    int32 count_invalid_names = 1;
+    bool show_debug_hint = 2;
+    string level = 3;
+}
+
+message TotalModelNamesWithSpacesDeprecationMsg {
+    CoreEventInfo info = 1;
+    TotalModelNamesWithSpacesDeprecation data = 2;
+}
+
 // I065
 message DeprecatedModel {
     string model_name = 1;

core/dbt/events/types.py

@@ -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}'
+
+
 # Event codes have prefixes which follow this table
 #
 # | Code |     Description     |
@@ -413,6 +418,43 @@
         return warning_tag(f"Deprecated functionality\n\n{description}")
 
 
+class SpacesInModelNameDeprecation(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."
+        )
+
+        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"Spaces in model names found in {self.count_invalid_names} model(s), which is deprecated."
+
+        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)
+
+
 # =======================================================
 # I - Project parsing
 # =======================================================

core/dbt/parser/manifest.py

@@ -22,6 +22,7 @@
 from dbt.context.manifest import generate_query_header_context
 from dbt.contracts.graph.semantic_manifest import SemanticManifest
 from dbt_common.events.base_types import EventLevel
+from dbt_common.exceptions.base import DbtValidationError
 import dbt_common.utils
 import json
 import pprint
@@ -62,6 +63,8 @@
     StateCheckVarsHash,
     DeprecatedModel,
     DeprecatedReference,
+    SpacesInModelNameDeprecation,
+    TotalModelNamesWithSpacesDeprecation,
     UpcomingReferenceDeprecation,
 )
 from dbt.logger import DbtProcessState
@@ -520,6 +523,7 @@
             self.write_manifest_for_partial_parse()
 
         self.check_for_model_deprecations()
+        self.check_for_spaces_in_model_names()
 
         return self.manifest
 
@@ -621,6 +625,47 @@
                             )
                         )
 
+    def check_for_spaces_in_model_names(self):
+        """Validates that model 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
+        """
+        improper_model_names = 0
+        level = (
+            EventLevel.WARN
+            if self.root_project.args.ALLOW_SPACES_IN_MODEL_NAMES
+            else EventLevel.ERROR
+        )
+
+        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:
+                    fire_event(
+                        SpacesInModelNameDeprecation(
+                            model_name=node.name,
+                            model_version=version_to_str(node.version),
+                            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")
+
     def load_and_parse_macros(self, project_parser_files):
         for project in self.all_projects.values():
             if project.project_name not in project_parser_files:

tests/functional/manifest_validations/test_check_for_spaces_in_model_names.py

@@ -0,0 +1,106 @@
+import pytest
+
+from dataclasses import dataclass, field
+from dbt.cli.main import dbtRunner
+from dbt_common.events.base_types import BaseEvent, EventLevel, EventMsg
+from dbt.events.types import SpacesInModelNameDeprecation, TotalModelNamesWithSpacesDeprecation
+from dbt.tests.util import update_config_file
+from typing import Dict, List
+
+
+@dataclass
+class EventCatcher:
+    event_to_catch: BaseEvent
+    caught_events: List[EventMsg] = field(default_factory=list)
+
+    def catch(self, event: EventMsg):
+        if event.info.name == self.event_to_catch.__name__:
+            self.caught_events.append(event)
+
+
+class TestSpacesInModelNamesHappyPath:
+    def test_no_warnings_when_no_spaces_in_name(self, project) -> None:
+        event_catcher = EventCatcher(SpacesInModelNameDeprecation)
+        runner = dbtRunner(callbacks=[event_catcher.catch])
+        runner.invoke(["parse"])
+        assert len(event_catcher.caught_events) == 0
+
+
+class TestSpacesInModelNamesSadPath:
+    @pytest.fixture(scope="class")
+    def models(self) -> Dict[str, str]:
+        return {
+            "my model.sql": "select 1 as id",
+        }
+
+    def tests_warning_when_spaces_in_name(self, project) -> None:
+        event_catcher = EventCatcher(SpacesInModelNameDeprecation)
+        total_catcher = EventCatcher(TotalModelNamesWithSpacesDeprecation)
+        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 event.info.level == EventLevel.WARN
+
+
+class TestSpaceInModelNamesWithDebug:
+    @pytest.fixture(scope="class")
+    def models(self) -> Dict[str, str]:
+        return {
+            "my model.sql": "select 1 as id",
+            "my model2.sql": "select 1 as id",
+        }
+
+    def tests_debug_when_spaces_in_name(self, project) -> None:
+        spaces_check_catcher = EventCatcher(SpacesInModelNameDeprecation)
+        total_catcher = EventCatcher(TotalModelNamesWithSpacesDeprecation)
+        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 (
+            "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)
+        runner = dbtRunner(callbacks=[spaces_check_catcher.catch, total_catcher.catch])
+        runner.invoke(["parse", "--debug"])
+        assert len(spaces_check_catcher.caught_events) == 2
+        assert len(total_catcher.caught_events) == 1
+        assert (
+            "Run again with `--debug` to see them all."
+            not in total_catcher.caught_events[0].info.msg
+        )
+
+
+class TestAllowSpacesInModelNamesFalse:
+    @pytest.fixture(scope="class")
+    def models(self) -> Dict[str, str]:
+        return {
+            "my model.sql": "select 1 as id",
+        }
+
+    def test_dont_allow_spaces_in_model_names(self, project):
+        spaces_check_catcher = EventCatcher(SpacesInModelNameDeprecation)
+        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}}
+        update_config_file(config_patch, project.project_root, "dbt_project.yml")
+
+        spaces_check_catcher = EventCatcher(SpacesInModelNameDeprecation)
+        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 len(spaces_check_catcher.caught_events) == 1
+        assert spaces_check_catcher.caught_events[0].info.level == EventLevel.ERROR

tests/unit/test_events.py

@@ -154,6 +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="", level=""),
+    core_types.TotalModelNamesWithSpacesDeprecation(
+        count_invalid_names=1, show_debug_hint=True, level=""
+    ),
     # E - DB Adapter ======================
     adapter_types.AdapterEventDebug(),
     adapter_types.AdapterEventInfo(),

tests/unit/test_graph.py

@@ -7,22 +7,22 @@
 from dbt.adapters.factory import reset_adapters, register_adapter
 import dbt.compilation
 import dbt.exceptions
-import dbt.flags
 import dbt.parser
 import dbt.config
 import dbt.utils
 import dbt.parser.manifest
 from dbt import tracking
+from dbt.cli.flags import convert_config
 from dbt.contracts.files import SourceFile, FileHash, FilePath
 from dbt.contracts.graph.manifest import MacroManifest, ManifestStateCheck
 from dbt.contracts.project import ProjectFlags
+from dbt.flags import get_flags, set_from_args
 from dbt.graph import NodeSelector, parse_difference
 from dbt.events.logging import setup_event_logger
 from dbt.mp_context import get_mp_context
 from queue import Empty
 from .utils import config_from_parts_or_dicts, generate_name_macros, inject_plugin
 
-from dbt.flags import set_from_args
 from argparse import Namespace
 
 set_from_args(Namespace(WARN_ERROR=False), None)
@@ -127,9 +127,15 @@ def get_config(self, extra_cfg=None):
         cfg.update(extra_cfg)
 
         config = config_from_parts_or_dicts(project=cfg, profile=self.profile)
-        dbt.flags.set_from_args(Namespace(), ProjectFlags())
-        setup_event_logger(dbt.flags.get_flags())
-        object.__setattr__(dbt.flags.get_flags(), "PARTIAL_PARSE", False)
+        set_from_args(Namespace(), ProjectFlags())
+        flags = get_flags()
+        setup_event_logger(flags)
+        object.__setattr__(flags, "PARTIAL_PARSE", False)
+        for arg_name, args_param_value in vars(flags).items():
+            args_param_value = convert_config(arg_name, args_param_value)
+            object.__setattr__(config.args, arg_name.upper(), args_param_value)
+            object.__setattr__(config.args, arg_name.lower(), args_param_value)
+
         return config
 
     def get_compiler(self, project):