Repository: TobikoData/sqlmesh
Pull request #2759 (2024-06-07 23:35:20)

Fix: Only report successful plan application when there's no exception


Patch:
diff --git a/sqlmesh/core/model/__init__.py b/sqlmesh/core/model/__init__.py
index e6bf7119..7860458d 100644
--- a/sqlmesh/core/model/__init__.py
+++ b/sqlmesh/core/model/__init__.py
@@ -4,6 +4,7 @@ from sqlmesh.core.model.cache import (
 )
 from sqlmesh.core.model.decorator import model as model
 from sqlmesh.core.model.definition import (
+    ExternalModel as ExternalModel,
     Model as Model,
     PythonModel as PythonModel,
     SeedModel as SeedModel,
diff --git a/sqlmesh/core/plan/builder.py b/sqlmesh/core/plan/builder.py
index 47240251..ee815fda 100644
--- a/sqlmesh/core/plan/builder.py
+++ b/sqlmesh/core/plan/builder.py
@@ -698,9 +698,9 @@ class PlanBuilder:
 
     def _ensure_no_broken_references(self) -> None:
         for snapshot in self._context_diff.snapshots.values():
-            broken_references = {x.name for x in self._context_diff.removed_snapshots} & {
-                x for x in snapshot.node.depends_on
-            }
+            broken_references = {
+                x.name for x in self._context_diff.removed_snapshots.values() if not x.is_external
+            } & {x for x in snapshot.node.depends_on}
             if broken_references:
                 broken_references_msg = ", ".join(f"'{x}'" for x in broken_references)
                 raise PlanError(
diff --git a/sqlmesh/core/plan/evaluator.py b/sqlmesh/core/plan/evaluator.py
index 1781495e..718ae07f 100644
--- a/sqlmesh/core/plan/evaluator.py
+++ b/sqlmesh/core/plan/evaluator.py
@@ -129,8 +129,9 @@ class BuiltInPlanEvaluator(PlanEvaluator):
         except Exception as e:
             analytics.collector.on_plan_apply_end(plan_id=plan.plan_id, error=e)
             raise
-        finally:
+        else:
             analytics.collector.on_plan_apply_end(plan_id=plan.plan_id)
+        finally:
             self.console.stop_plan_evaluation()
 
     def _backfill(


Test patch:
diff --git a/tests/core/test_plan.py b/tests/core/test_plan.py
index 23ba84c4..990135ca 100644
--- a/tests/core/test_plan.py
+++ b/tests/core/test_plan.py
@@ -13,6 +13,7 @@ from sqlmesh.core.context_diff import ContextDiff
 from sqlmesh.core.engine_adapter import DuckDBEngineAdapter
 from sqlmesh.core.environment import EnvironmentNamingInfo
 from sqlmesh.core.model import (
+    ExternalModel,
     FullKind,
     IncrementalByTimeRangeKind,
     SeedKind,
@@ -1163,6 +1164,36 @@ def test_broken_references(make_snapshot, mocker: MockerFixture):
         PlanBuilder(context_diff, DuckDBEngineAdapter.SCHEMA_DIFFER).build()
 
 
+def test_broken_references_external_model(make_snapshot, mocker: MockerFixture):
+    snapshot_a = make_snapshot(ExternalModel(name="a", kind=dict(name=ModelKindName.EXTERNAL)))
+    snapshot_a.categorize_as(SnapshotChangeCategory.BREAKING)
+
+    snapshot_b = make_snapshot(SqlModel(name="b", query=parse_one("select 2, ds FROM a")))
+    snapshot_b.categorize_as(SnapshotChangeCategory.BREAKING)
+
+    context_diff = ContextDiff(
+        environment="test_environment",
+        is_new_environment=True,
+        is_unfinalized_environment=False,
+        create_from="prod",
+        added=set(),
+        removed_snapshots={snapshot_a.snapshot_id: snapshot_a.table_info},
+        modified_snapshots={snapshot_b.name: (snapshot_b, snapshot_b)},
+        snapshots={snapshot_b.snapshot_id: snapshot_b},
+        new_snapshots={},
+        previous_plan_id=None,
+        previously_promoted_snapshot_ids=set(),
+        previous_finalized_snapshots=None,
+    )
+
+    # Make sure the downstream snapshot doesn't have any parents,
+    # since its only parent has been removed from the project.
+    assert not snapshot_b.parents
+
+    # Shouldn't raise
+    PlanBuilder(context_diff, DuckDBEngineAdapter.SCHEMA_DIFFER).build()
+
+
 def test_effective_from(make_snapshot, mocker: MockerFixture):
     snapshot = make_snapshot(
         SqlModel(