Chore: Improve error messages return to a user when there are models with the same name (#4425)

Author: izeigerman

sqlmesh/core/loader.py

@@ -223,7 +223,7 @@ def _load_external_models(
         if external_models_path.exists() and external_models_path.is_dir():
             paths_to_load.extend(self._glob_paths(external_models_path, extension=".yaml"))
 
-        def _load() -> t.List[Model]:
+        def _load(path: Path) -> t.List[Model]:
             try:
                 with open(path, "r", encoding="utf-8") as file:
                     return [
@@ -241,21 +241,30 @@ def _load() -> t.List[Model]:
                         for row in YAML().load(file.read())
                     ]
             except Exception as ex:
-                raise ConfigError(f"Failed to load model definition at '{path}'.\n{ex}")
+                self._raise_failed_to_load_model_error(path, str(ex))
+                raise
 
         for path in paths_to_load:
             self._track_file(path)
 
-            external_models = cache.get_or_load_models(path, _load)
+            external_models = cache.get_or_load_models(path, lambda: _load(path))
             # external models with no explicit gateway defined form the base set
             for model in external_models:
                 if model.gateway is None:
+                    if model.fqn in models:
+                        self._raise_failed_to_load_model_error(
+                            path, f"Duplicate external model name: '{model.name}'."
+                        )
                     models[model.fqn] = model
 
             # however, if there is a gateway defined, gateway-specific models take precedence
             if gateway:
                 for model in external_models:
                     if model.gateway == gateway:
+                        if model.fqn in models and models[model.fqn].gateway == gateway:
+                            self._raise_failed_to_load_model_error(
+                                path, f"Duplicate external model name: '{model.name}'."
+                            )
                         models.update({model.fqn: model})
 
         return models
@@ -363,6 +372,9 @@ def _get_variables(self, gateway_name: t.Optional[str] = None) -> t.Dict[str, t.
 
         return self._variables_by_gateway[gateway_name]
 
+    def _raise_failed_to_load_model_error(self, path: Path, message: str) -> None:
+        raise ConfigError(f"Failed to load model definition at '{path}'.\n{message}")
+
 
 class SqlMeshLoader(Loader):
     """Loads macros and models for a context using the SQLMesh file formats"""
@@ -484,9 +496,14 @@ def _load() -> t.List[Model]:
                         default_catalog_per_gateway=self.context.default_catalog_per_gateway,
                     )
                 except Exception as ex:
-                    raise ConfigError(f"Failed to load model definition at '{path}'.\n{ex}")
+                    self._raise_failed_to_load_model_error(path, str(ex))
+                    raise
 
             for model in cache.get_or_load_models(path, _load):
+                if model.fqn in models:
+                    self._raise_failed_to_load_model_error(
+                        path, f"Duplicate SQL model name: '{model.name}'."
+                    )
                 if model.enabled:
                     models[model.fqn] = model
 
@@ -544,7 +561,7 @@ def _load_python_models(
                             if model.enabled:
                                 models[model.fqn] = model
                 except Exception as ex:
-                    raise ConfigError(f"Failed to load model definition at '{path}'.\n{ex}")
+                    self._raise_failed_to_load_model_error(path, str(ex))
 
         finally:
             model_registry._dialect = None

sqlmesh/utils/__init__.py

@@ -136,7 +136,7 @@ def __call__(
         except ValueError:
             # No need to raise due to duplicate key if the functions are identical
             if func.__code__.co_code != registry[func_name].func.__code__.co_code:
-                raise
+                raise ValueError(f"Duplicate name: '{func_name}'.")
 
         @wraps(func)
         def wrapper(*args: t.Any, **kwargs: t.Any) -> DECORATOR_RETURN_TYPE:

tests/core/test_loader.py

@@ -109,8 +109,8 @@ def duplicate_model_path(fpath):
     duplicate_fpath.write_text(model["contents"])
 
     with pytest.raises(
-        ValueError,
-        match=r'Duplicate key \'"memory"."test_schema"."test_model"\' found in UniqueKeyDict<models>. Call dict.update\(\.\.\.\) if this is intentional.',
+        ConfigError,
+        match=r".*Duplicate .* model name: 'test_schema.test_model'",
     ):
         Context(paths=tmp_path, config=config)
 
@@ -159,7 +159,7 @@ def execute(
 
     with pytest.raises(
         ConfigError,
-        match=r"Failed to load model definition at '.*'.\nDuplicate key 'test_schema.test_model' found in UniqueKeyDict<python_models>.",
+        match=r"Failed to load model definition at '.*'.\nDuplicate name: 'test_schema.test_model'.",
     ):
         Context(paths=tmp_path, config=config)