Fix: Reporting deletion of physical tables for snapshots of symbolic / audit models (#5422)

Author: izeigerman

sqlmesh/core/snapshot/evaluator.py

@@ -518,10 +518,12 @@ def cleanup(
             target_snapshots: Snapshots to cleanup.
             on_complete: A callback to call on each successfully deleted database object.
         """
+        target_snapshots = [
+            t for t in target_snapshots if t.snapshot.is_model and not t.snapshot.is_symbolic
+        ]
         snapshots_to_dev_table_only = {
             t.snapshot.snapshot_id: t.dev_table_only for t in target_snapshots
         }
-
         with self.concurrent_context():
             concurrent_apply_to_snapshots(
                 [t.snapshot for t in target_snapshots],

tests/core/test_snapshot_evaluator.py

@@ -436,10 +436,14 @@ def create_and_cleanup(name: str, dev_table_only: bool):
         snapshot.categorize_as(SnapshotChangeCategory.BREAKING, forward_only=True)
         snapshot.version = "test_version"
 
+        on_cleanup_mock = mocker.Mock()
+
         evaluator.promote([snapshot], EnvironmentNamingInfo(name="test_env"))
         evaluator.cleanup(
-            [SnapshotTableCleanupTask(snapshot=snapshot.table_info, dev_table_only=dev_table_only)]
+            [SnapshotTableCleanupTask(snapshot=snapshot.table_info, dev_table_only=dev_table_only)],
+            on_complete=on_cleanup_mock,
         )
+        assert on_cleanup_mock.call_count == 1 if dev_table_only else 2
         return snapshot
 
     snapshot = create_and_cleanup("catalog.test_schema.test_model", True)
@@ -611,6 +615,39 @@ def create_and_cleanup_external_model(name: str, dev_table_only: bool):
     adapter_mock.drop_table.assert_not_called()
 
 
+def test_cleanup_symbolic_and_audit_snapshots_no_callback(
+    mocker: MockerFixture, adapter_mock, make_snapshot
+):
+    evaluator = SnapshotEvaluator(adapter_mock)
+    on_complete_mock = mocker.Mock()
+
+    # Test external model
+    external_model = ExternalModel(
+        name="test_schema.external_model",
+        kind=ExternalKind(),
+    )
+    external_snapshot = make_snapshot(external_model)
+    external_snapshot.categorize_as(SnapshotChangeCategory.BREAKING)
+
+    # Test standalone audit
+    audit = StandaloneAudit(name="test_audit", query=parse_one("SELECT NULL LIMIT 0"))
+    audit_snapshot = make_snapshot(audit)
+    audit_snapshot.categorize_as(SnapshotChangeCategory.NON_BREAKING)
+
+    evaluator.cleanup(
+        [
+            SnapshotTableCleanupTask(snapshot=external_snapshot.table_info, dev_table_only=False),
+            SnapshotTableCleanupTask(snapshot=audit_snapshot.table_info, dev_table_only=False),
+        ],
+        on_complete=on_complete_mock,
+    )
+
+    # Verify that no physical tables were attempted to be dropped
+    adapter_mock.drop_table.assert_not_called()
+    adapter_mock.get_data_object.assert_not_called()
+    on_complete_mock.assert_not_called()
+
+
 @pytest.mark.parametrize("view_exists", [True, False])
 def test_evaluate_materialized_view(
     mocker: MockerFixture, adapter_mock, make_snapshot, view_exists: bool

tests/integrations/jupyter/test_magics.py

@@ -906,9 +906,6 @@ def test_destroy(
         "Are you ABSOLUTELY SURE you want to proceed with deletion? [y/n]:",
         "Environment 'prod' invalidated.",
         "Deleted object memory.sushi",
-        'Deleted object "memory"."raw"."model1"',
-        'Deleted object "memory"."raw"."model2"',
-        'Deleted object "memory"."raw"."demographics"',
         "State tables removed.",
         "Destroy completed successfully.",
     ]