Fokko commented on code in PR #728:
URL: https://github.com/apache/iceberg-python/pull/728#discussion_r1624116714


##########
pyiceberg/table/__init__.py:
##########
@@ -277,6 +279,50 @@ def __init__(self, table: Table, autocommit: bool = False):
         self._autocommit = autocommit
         self._updates = ()
         self._requirements = ()
+        self._manage_snapshots = Transaction.ManageSnapshot(self)
+
+    class ManageSnapshot:

Review Comment:
   Thanks for splitting this into a separate class. I think that's a great idea 
and also reflects the 
[UpdateSnapshotReferencesOperation](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/UpdateSnapshotReferencesOperation.java)
 in Java. Here they keep state inside of the transaction, so there you can do:
   
   ```python
   with table.manage_snapshots() as ms:
          ms.create_tag("production-3-6-2024", 
table.current_snapshot().snapshot_id)
          ms.drop_tag("production-3-6-2024")
   ```
   
   This would then result in a no-op. We don't have to do this for this first 
iteration.
   
   > 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
   
   I'm unsure where the `AssertTableUUID` comes from, since I don't think 
that's required when updating a ref. Looks like this isn't checked in Java: 
https://github.com/apache/iceberg/blob/252168419f8ac1d251b39f0944a189184056e543/core/src/main/java/org/apache/iceberg/UpdateRequirements.java#L113-L125
   



-- 
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