ForeverAngry commented on code in PR #1880:
URL: https://github.com/apache/iceberg-python/pull/1880#discussion_r2057106903


##########
pyiceberg/table/update/snapshot.py:
##########
@@ -843,3 +849,64 @@ def remove_branch(self, branch_name: str) -> 
ManageSnapshots:
             This for method chaining
         """
         return self._remove_ref_snapshot(ref_name=branch_name)
+
+class ExpireSnapshots(UpdateTableMetadata["ExpireSnapshots"]):
+    """
+    Expire snapshots by ID or by timestamp.
+    Use table.expire_snapshots().<operation>().commit() to run a specific 
operation.
+    Use table.expire_snapshots().<operation-one>().<operation-two>().commit() 
to run multiple operations.
+    Pending changes are applied on commit.
+    """
+
+    _snapshot_ids_to_expire = set()
+    _updates: Tuple[TableUpdate, ...] = ()
+    _requirements: Tuple[TableRequirement, ...] = ()
+
+    def _commit(self) -> UpdatesAndRequirements:
+        """
+        Commit the staged updates and requirements.
+        This will remove the snapshots with the given IDs.
+
+        Returns:
+            Tuple of updates and requirements to be committed,
+            as required by the calling parent apply functions.
+        """
+        update = 
RemoveSnapshotsUpdate(snapshot_ids=self._snapshot_ids_to_expire)
+        self._updates += (update,)
+        return self._updates, self._requirements
+
+    def expire_snapshot_by_id(self, snapshot_id: int) -> ExpireSnapshots:
+        """
+        Expire a snapshot by its ID.
+
+        Args:
+            snapshot_id (int): The ID of the snapshot to expire.
+
+        Returns:
+            This for method chaining.
+        """
+        if self._transaction.table_metadata.snapshot_by_id(snapshot_id) is 
None:
+            raise ValueError(f"Snapshot with ID {snapshot_id} does not exist.")
+        self._snapshot_ids_to_expire.add(snapshot_id)
+        return self
+
+    def expire_snapshots_older_than(self, timestamp_ms: int) -> 
ExpireSnapshots:
+        """
+        Expire snapshots older than the given timestamp.
+
+        Args:
+            timestamp_ms (int): The timestamp in milliseconds. Snapshots older 
than this will be expired.

Review Comment:
   To help make this a bit more clear, i think id like to do a separate issue / 
pr for the `expire_snapshots_older_than`. Thoughts? 
   



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