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

Reply via email to