chinmay-bhat commented on code in PR #728: URL: https://github.com/apache/iceberg-python/pull/728#discussion_r1624084300
########## pyiceberg/table/__init__.py: ########## @@ -1806,6 +1891,85 @@ def __enter__(self) -> U: return self # type: ignore +class ManageSnapshots(UpdateTableMetadata["ManageSnapshots"]): + """Run snapshot management operations using APIs.""" + + _updates: Tuple[TableUpdate, ...] = () + _requirements: Tuple[TableRequirement, ...] = () + _parent_snapshot_id: Optional[int] + + def _commit(self) -> UpdatesAndRequirements: + """Apply the pending changes and commit.""" + if self._updates: + self._requirements += ( + AssertRefSnapshotId(snapshot_id=self._parent_snapshot_id, ref="main"), + AssertTableUUID(uuid=self._transaction.table_metadata.table_uuid), + ) Review Comment: Linking relevant [comment](https://github.com/apache/iceberg-python/pull/728#discussion_r1611114716) ``` > We could do multiple tags and branch creations and then _apply once to reduce the number of transaction._apply() call since that will trigger requirement validations every time. This also allow us to avoid duplicated requirements being added such as AssertTableUUID ``` That's why I've added it here, instead of _set_ref_snapshot ########## pyiceberg/table/__init__.py: ########## @@ -1806,6 +1891,85 @@ def __enter__(self) -> U: return self # type: ignore +class ManageSnapshots(UpdateTableMetadata["ManageSnapshots"]): + """Run snapshot management operations using APIs.""" + + _updates: Tuple[TableUpdate, ...] = () + _requirements: Tuple[TableRequirement, ...] = () + _parent_snapshot_id: Optional[int] + + def _commit(self) -> UpdatesAndRequirements: + """Apply the pending changes and commit.""" + if self._updates: + self._requirements += ( + AssertRefSnapshotId(snapshot_id=self._parent_snapshot_id, ref="main"), + AssertTableUUID(uuid=self._transaction.table_metadata.table_uuid), + ) Review Comment: Linking relevant [comment](https://github.com/apache/iceberg-python/pull/728#discussion_r1611114716) > We could do multiple tags and branch creations and then _apply once to reduce the number of transaction._apply() call since that will trigger requirement validations every time. This also allow us to avoid duplicated requirements being added such as AssertTableUUID That's why I've added it here, instead of _set_ref_snapshot -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org