rebase; cleanup code

Author: themisvaltinos

Makefile

@@ -64,7 +64,7 @@ engine-up: engine-clickhouse-up engine-mssql-up engine-mysql-up engine-postgres-
 engine-down: engine-clickhouse-down engine-mssql-down engine-mysql-down engine-postgres-down engine-spark-down engine-trino-down
 
 fast-test:
-	pytest -n auto -m "fast and not cicdonly" && pytest -m "isolated"
+	pytest -n auto -m "fast and not cicdonly" && pytest -m "isolated" && pytest -m "isolated2"
 
 slow-test:
 	pytest -n auto -m "(fast or slow) and not cicdonly" && pytest -m "isolated"

sqlmesh/core/loader.py

@@ -313,8 +313,10 @@ def _load_external_models(
             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}'."
+                        raise ConfigError(
+                            self._failed_to_load_model_error(
+                                path, f"Duplicate external model name: '{model.name}'."
+                            )
                         )
                     models[model.fqn] = model
 
@@ -323,8 +325,10 @@ def _load_external_models(
                 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}'."
+                            raise ConfigError(
+                                self._failed_to_load_model_error(
+                                    path, f"Duplicate external model name: '{model.name}'."
+                                )
                             )
                         models.update({model.fqn: model})
 
@@ -411,11 +415,12 @@ def _track_file(self, path: Path) -> None:
         """Project file to track for modifications"""
         self._path_mtimes[path] = path.stat().st_mtime
 
-    def _raise_failed_to_load_model_error(self, path: Path, error: t.Union[str, Exception]) -> None:
+    def _failed_to_load_model_error(self, path: Path, error: t.Union[str, Exception]) -> str:
         base_message = f"Failed to load model definition at '{path}':"
         if isinstance(error, ValidationError):
-            raise ConfigError(validation_error_message(error, base_message))
-        raise ConfigError(f"{base_message}\n  {error}")
+            return validation_error_message(error, base_message)
+        return f"{base_message}\n  {error}"
+
 
 class SqlMeshLoader(Loader):
     """Loads macros and models for a context using the SQLMesh file formats"""
@@ -552,17 +557,18 @@ def _load_sql_models(
                     path = futures_to_paths[future]
                     try:
                         _, loaded = future.result()
-                        if loaded:
-                            for model in loaded:
-                                if model.enabled:
-                                    model._path = path
-                                    models[model.fqn] = model
-                        else:
-                            for model in cache.get(path):
-                                if model.enabled:
-                                    models[model.fqn] = model
+                        for model in loaded or cache.get(path):
+                            if model.fqn in models:
+                                errors.append(
+                                    self._failed_to_load_model_error(
+                                        path, f"Duplicate SQL model name: '{model.name}'."
+                                    )
+                                )
+                            elif model.enabled:
+                                model._path = path
+                                models[model.fqn] = model
                     except Exception as ex:
-                        errors.append(f"Failed to load model definition at '{path}'.\n\n{ex}")
+                        errors.append(self._failed_to_load_model_error(path, str(ex)))
 
             if errors:
                 error_string = "\n".join(errors)
@@ -618,7 +624,7 @@ def _load_python_models(
                             if model.enabled:
                                 models[model.fqn] = model
                 except Exception as ex:
-                    self._raise_failed_to_load_model_error(path, ex)
+                    raise ConfigError(self._failed_to_load_model_error(path, ex))
 
         finally:
             model_registry._dialect = None

tests/conftest.py

@@ -183,7 +183,7 @@ def validate(
 
 
 def pytest_collection_modifyitems(items, *args, **kwargs):
-    test_type_markers = {"fast", "slow", "docker", "remote", "isolated"}
+    test_type_markers = {"fast", "slow", "docker", "remote", "isolated", "isolated2"}
     for item in items:
         for marker in item.iter_markers():
             if marker.name in test_type_markers:

tests/core/test_loader.py

@@ -115,7 +115,7 @@ def duplicate_model_path(fpath):
         Context(paths=tmp_path, config=config)
 
 
-@pytest.mark.isolated
+@pytest.mark.isolated2
 def test_duplicate_python_model_names_raise_error(tmp_path: Path) -> None:
     """Test python models with duplicate model names raises ConfigError if the functions are not identical."""
     init_example_project(tmp_path, dialect="duckdb")