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: [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]
