lawofcycles commented on code in PR #3320:
URL: https://github.com/apache/iceberg-python/pull/3320#discussion_r3603736301


##########
pyiceberg/table/update/snapshot.py:
##########
@@ -91,19 +97,50 @@ def _new_manifest_list_file_name(snapshot_id: int, attempt: 
int, commit_uuid: uu
     return f"snap-{snapshot_id}-{attempt}-{commit_uuid}.avro"
 
 
+@dataclass(frozen=True)
+class CommitWindow:
+    """Tracks the commit range to validate against during retry.
+
+    base: The snapshot when the operation began (exclusive lower bound, fixed 
across retries).
+    head: The branch HEAD snapshot after metadata refresh (inclusive upper 
bound).
+    """
+
+    base: Snapshot | None
+    head: Snapshot | None
+
+    @classmethod
+    def resolve(cls, metadata: TableMetadata, base_id: int | None, branch: str 
| None) -> CommitWindow:
+        """Resolve a CommitWindow from metadata, starting snapshot ID, and 
target branch."""
+        head = metadata.snapshot_by_name(branch)
+        base = metadata.snapshot_by_id(base_id) if base_id is not None else 
None
+        if base_id is not None and base is None:
+            raise ValidationException(f"Cannot find starting snapshot 
{base_id}")

Review Comment:
   I think this branch is actually reachable.
   
   `snapshot_by_id` returns `None` rather than raising here, since it is 
implemented as `next((...), None)`, so execution does reach the guard.
   
   `base_id` is the branch head snapshot id captured when the operation starts 
(`_starting_snapshot_id`), fixed across retries. On retry we look it up in the 
refreshed metadata. It resolves to `None` when the starting snapshot has been 
expired or removed between the start of the operation and the retry. In that 
case we can no longer trace the validation ancestry from it, so raising 
`ValidationException` is the safe behavior.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to