PR feedback

Author: erindru

sqlmesh/core/snapshot/__init__.py

@@ -4,7 +4,7 @@
     Node as Node,
     QualifiedViewName as QualifiedViewName,
     Snapshot as Snapshot,
-    MinimalSnapshot as MinimalSnapshot,
+    SnapshotIdAndVersion as SnapshotIdAndVersion,
     SnapshotChangeCategory as SnapshotChangeCategory,
     SnapshotDataVersion as SnapshotDataVersion,
     SnapshotFingerprint as SnapshotFingerprint,

sqlmesh/core/snapshot/definition.py

@@ -588,7 +588,7 @@ def name_version(self) -> SnapshotNameVersion:
         return SnapshotNameVersion(name=self.name, version=self.version)
 
 
-class MinimalSnapshot(PydanticModel):
+class SnapshotIdAndVersion(PydanticModel):
     """A stripped down version of a snapshot that is used in situations where we want to fetch the main fields of the snapshots table
     without the overhead of parsing the full snapshot payload and fetching intervals.
     """
@@ -597,7 +597,7 @@ class MinimalSnapshot(PydanticModel):
     version: str
     dev_version_: t.Optional[str] = Field(alias="dev_version")
     identifier: str
-    fingerprint: SnapshotFingerprint
+    fingerprint_: t.Union[str, SnapshotFingerprint] = Field(alias="fingerprint")
 
     @property
     def snapshot_id(self) -> SnapshotId:
@@ -607,6 +607,13 @@ def snapshot_id(self) -> SnapshotId:
     def name_version(self) -> SnapshotNameVersion:
         return SnapshotNameVersion(name=self.name, version=self.version)
 
+    @property
+    def fingerprint(self) -> SnapshotFingerprint:
+        value = self.fingerprint_
+        if isinstance(value, str):
+            self.fingerprint_ = value = SnapshotFingerprint.parse_raw(value)
+        return value
+
     @property
     def dev_version(self) -> str:
         return self.dev_version_ or self.fingerprint.to_version()
@@ -1487,9 +1494,11 @@ class SnapshotTableCleanupTask(PydanticModel):
     dev_table_only: bool
 
 
-SnapshotIdLike = t.Union[SnapshotId, SnapshotTableInfo, MinimalSnapshot, Snapshot]
+SnapshotIdLike = t.Union[SnapshotId, SnapshotTableInfo, SnapshotIdAndVersion, Snapshot]
 SnapshotInfoLike = t.Union[SnapshotTableInfo, Snapshot]
-SnapshotNameVersionLike = t.Union[SnapshotNameVersion, SnapshotTableInfo, MinimalSnapshot, Snapshot]
+SnapshotNameVersionLike = t.Union[
+    SnapshotNameVersion, SnapshotTableInfo, SnapshotIdAndVersion, Snapshot
+]
 
 
 class DeployabilityIndex(PydanticModel, frozen=True):

sqlmesh/core/state_sync/base.py

@@ -23,7 +23,7 @@
     SnapshotTableCleanupTask,
     SnapshotTableInfo,
     SnapshotNameVersion,
-    MinimalSnapshot,
+    SnapshotIdAndVersion,
 )
 from sqlmesh.core.snapshot.definition import Interval, SnapshotIntervals
 from sqlmesh.utils import major_minor
@@ -104,7 +104,7 @@ def get_snapshots_by_names(
         snapshot_names: t.Iterable[str],
         current_ts: t.Optional[int] = None,
         exclude_expired: bool = True,
-    ) -> t.Set[MinimalSnapshot]:
+    ) -> t.Set[SnapshotIdAndVersion]:
         """Return the snapshot records for all versions of the specified snapshot names.
 
         Args:

sqlmesh/core/state_sync/db/facade.py

@@ -29,7 +29,7 @@
 from sqlmesh.core.environment import Environment, EnvironmentStatements, EnvironmentSummary
 from sqlmesh.core.snapshot import (
     Snapshot,
-    MinimalSnapshot,
+    SnapshotIdAndVersion,
     SnapshotId,
     SnapshotIdLike,
     SnapshotInfoLike,
@@ -372,7 +372,7 @@ def get_snapshots_by_names(
         snapshot_names: t.Iterable[str],
         current_ts: t.Optional[int] = None,
         exclude_expired: bool = True,
-    ) -> t.Set[MinimalSnapshot]:
+    ) -> t.Set[SnapshotIdAndVersion]:
         return self.snapshot_state.get_snapshots_by_names(
             snapshot_names=snapshot_names, current_ts=current_ts, exclude_expired=exclude_expired
         )

sqlmesh/core/state_sync/db/snapshot.py

@@ -26,7 +26,7 @@
     SnapshotNameVersion,
     SnapshotInfoLike,
     Snapshot,
-    MinimalSnapshot,
+    SnapshotIdAndVersion,
     SnapshotId,
     SnapshotFingerprint,
 )
@@ -214,7 +214,7 @@ def _get_expired_snapshots(
             for snapshot in environment.snapshots
         }
 
-        def _is_snapshot_used(snapshot: MinimalSnapshot) -> bool:
+        def _is_snapshot_used(snapshot: SnapshotIdAndVersion) -> bool:
             return (
                 snapshot.snapshot_id in promoted_snapshot_ids
                 or snapshot.snapshot_id not in expired_candidates
@@ -312,7 +312,7 @@ def get_snapshots_by_names(
         snapshot_names: t.Iterable[str],
         current_ts: t.Optional[int] = None,
         exclude_expired: bool = True,
-    ) -> t.Set[MinimalSnapshot]:
+    ) -> t.Set[SnapshotIdAndVersion]:
         """Return the snapshot records for all versions of the specified snapshot names.
 
         Args:
@@ -333,12 +333,12 @@ def get_snapshots_by_names(
             unexpired_expr = None
 
         return {
-            MinimalSnapshot(
+            SnapshotIdAndVersion(
                 name=name,
                 identifier=identifier,
                 version=version,
                 dev_version=dev_version,
-                fingerprint=SnapshotFingerprint.parse_raw(fingerprint),
+                fingerprint=fingerprint,
             )
             for where in snapshot_name_filter(
                 snapshot_names=snapshot_names,
@@ -636,7 +636,7 @@ def _get_snapshots_with_same_version(
         self,
         snapshots: t.Collection[SnapshotNameVersionLike],
         lock_for_update: bool = False,
-    ) -> t.List[MinimalSnapshot]:
+    ) -> t.List[SnapshotIdAndVersion]:
         """Fetches all snapshots that share the same version as the snapshots.
 
         The output includes the snapshots with the specified identifiers.
@@ -673,7 +673,7 @@ def _get_snapshots_with_same_version(
             snapshot_rows.extend(fetchall(self.engine_adapter, query))
 
         return [
-            MinimalSnapshot(
+            SnapshotIdAndVersion(
                 name=name,
                 identifier=identifier,
                 version=version,

tests/core/test_snapshot.py

@@ -44,6 +44,7 @@
     QualifiedViewName,
     Snapshot,
     SnapshotId,
+    SnapshotIdAndVersion,
     SnapshotChangeCategory,
     SnapshotFingerprint,
     SnapshotIntervals,
@@ -3532,3 +3533,35 @@ def test_table_name_virtual_environment_mode(
             assert table_name_result.endswith(snapshot.version)
         else:
             assert table_name_result.endswith(f"{snapshot.dev_version}__dev")
+
+
+def test_snapshot_id_and_version_fingerprint_lazy_init():
+    snapshot = SnapshotIdAndVersion(
+        name="a",
+        identifier="1234",
+        version="2345",
+        dev_version=None,
+        fingerprint='{"data_hash":"1","metadata_hash":"2","parent_data_hash":"3","parent_metadata_hash":"4"}',
+    )
+
+    # starts off as a string in the private property
+    assert isinstance(snapshot.fingerprint_, str)
+
+    # gets parsed into SnapshotFingerprint on first access of public property
+    fingerprint = snapshot.fingerprint
+    assert isinstance(fingerprint, SnapshotFingerprint)
+    assert isinstance(snapshot.fingerprint_, SnapshotFingerprint)
+
+    assert fingerprint.data_hash == "1"
+    assert fingerprint.metadata_hash == "2"
+    assert fingerprint.parent_data_hash == "3"
+    assert fingerprint.parent_metadata_hash == "4"
+    assert snapshot.dev_version is not None  # dev version uses fingerprint
+
+    # can also be supplied as a SnapshotFingerprint to begin with instead of a str
+    snapshot = SnapshotIdAndVersion(
+        name="a", identifier="1234", version="2345", dev_version=None, fingerprint=fingerprint
+    )
+
+    assert isinstance(snapshot.fingerprint_, SnapshotFingerprint)
+    assert snapshot.fingerprint == fingerprint