rambleraptor commented on code in PR #3320:
URL: https://github.com/apache/iceberg-python/pull/3320#discussion_r3438721997
##########
tests/table/test_commit_retry.py:
##########
@@ -0,0 +1,724 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
Can we get some tests that don't occur on the main branch? I want to make
sure that we're properly tracking the non-main branch commits.
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -91,19 +96,35 @@ def _new_manifest_list_file_name(snapshot_id: int, attempt:
int, commit_uuid: uu
return f"snap-{snapshot_id}-{attempt}-{commit_uuid}.avro"
+@dataclass
+class CommitWindow:
Review Comment:
I should've been way more clear about what I had in mind with CommitWindow.
I was hoping to take all of the commit logic and abstract it into a single
place. I wrote up some code to make it easier to understand:
```
@dataclass(frozen=True)
class CommitWindow:
base: Snapshot | None # exclusive lower bound: transaction start
head: Snapshot | None # inclusive upper bound: branch head after
refresh
@classmethod
def resolve(cls, metadata: TableMetadata, base_id: int | None, branch:
str | None) -> CommitWindow:
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}")
return cls(base=base, head=head)
def is_empty(self) -> bool:
return self.head is None or self.base is None or
self.base.snapshot_id == self.head.snapshot_id
```
This would handle the three _resolve methods that are spread out. Bonus, we
can reuse this elsewhere!
--
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]
