chinmay-bhat commented on code in PR #728:
URL: https://github.com/apache/iceberg-python/pull/728#discussion_r1614633223
##########
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:
> I am thinking that ManageSnapshots can be an outer class like
[UpdaetSchema](https://github.com/apache/iceberg-python/blob/55bbc1863aba6646b2dc36573355aabb11909095/pyiceberg/table/__init__.py#L1920)
and implements the UpdateTableMetadata:
I agree, that's a great catch. I've now updated the class.
> We could utilize Transaction's autocommit option when implementing
table.manage_snapshots, like:
Updated!
> We could also make create_tag returns ManageSnapshots itself to support
chain operations:
This is great, updated to allow method chaining!
> There are several benefits:
I do agree with the benefits. However, with this change, `ManageSnapshots`
would be specific to metadata update operations only. In my other PRs, I've
implemented `cherrypick` and `publish_changes`, both of which need to create a
new snapshot from an existing snapshot using `_MergingSnapshotProducer`. I'm
assuming we would like to have these APIs under `ManageSnapshots` like
`table.manage_snapshots().cherrypick(snapshot_id)`.
Ex:
```
class ManageSnapshots:
def cherrypick(self, snapshot_id: int) -> ManageSnapshots:
CherryPickSnapshot(snapshot_id, .....).cherrypick()
return self
```
Here, we are performing cherrypick through a class CherryPickSnapshot that
implements _MergingSnapshotProducer. So no updates or requirements are added to
ManageSnapshots, so ManageSnapshots.commit() is never used. Would this be a
reasonable implementation?
--
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
