-
Notifications
You must be signed in to change notification settings - Fork 419
feat: Add Set Current Snapshot to ManageSnapshots API #2871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -843,6 +843,13 @@ def _commit(self) -> UpdatesAndRequirements: | |
| """Apply the pending changes and commit.""" | ||
| return self._updates, self._requirements | ||
|
|
||
| def _commit_if_ref_updates_exist(self) -> None: | ||
| """Stage any pending ref updates to the transaction state.""" | ||
| if self._updates: | ||
| self._transaction._stage(*self._commit()) | ||
| self._updates = () | ||
| self._requirements = () | ||
|
|
||
| def _remove_ref_snapshot(self, ref_name: str) -> ManageSnapshots: | ||
| """Remove a snapshot ref. | ||
|
|
||
|
|
@@ -941,6 +948,43 @@ def remove_branch(self, branch_name: str) -> ManageSnapshots: | |
| """ | ||
| return self._remove_ref_snapshot(ref_name=branch_name) | ||
|
|
||
| def set_current_snapshot(self, snapshot_id: int | None = None, ref_name: str | None = None) -> ManageSnapshots: | ||
| """Set the current snapshot to a specific snapshot ID or ref. | ||
|
|
||
| Args: | ||
| snapshot_id: The ID of the snapshot to set as current. | ||
| ref_name: The snapshot reference (branch or tag) to set as current. | ||
|
|
||
| Returns: | ||
| This for method chaining. | ||
|
|
||
| Raises: | ||
| ValueError: If neither or both arguments are provided, or if the snapshot/ref does not exist. | ||
| """ | ||
| self._commit_if_ref_updates_exist() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. awesome! this follows the java SnapshotManager implementation and allows creating a tag and set the current snapshot as the tag in the same catalog commit (i.e. in the same transaction) |
||
|
|
||
| if (snapshot_id is None) == (ref_name is None): | ||
| raise ValueError("Either snapshot_id or ref_name must be provided, not both") | ||
|
|
||
| target_snapshot_id: int | ||
| if snapshot_id is not None: | ||
| target_snapshot_id = snapshot_id | ||
| else: | ||
| if ref_name not in self._transaction.table_metadata.refs: | ||
| raise ValueError(f"Cannot find matching snapshot ID for ref: {ref_name}") | ||
| target_snapshot_id = self._transaction.table_metadata.refs[ref_name].snapshot_id | ||
|
|
||
| if self._transaction.table_metadata.snapshot_by_id(target_snapshot_id) is None: | ||
| raise ValueError(f"Cannot set current snapshot to unknown snapshot id: {target_snapshot_id}") | ||
|
|
||
| update, requirement = self._transaction._set_ref_snapshot( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can rip this helper out of transaction and leave in managesnapshots api. |
||
| snapshot_id=target_snapshot_id, | ||
| ref_name=MAIN_BRANCH, | ||
| type=SnapshotRefType.BRANCH, | ||
| ) | ||
| self._transaction._stage(update, requirement) | ||
| return self | ||
|
|
||
|
|
||
| class ExpireSnapshots(UpdateTableMetadata["ExpireSnapshots"]): | ||
| """Expire snapshots by ID. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,179 @@ | ||
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| from unittest.mock import MagicMock | ||
| from uuid import uuid4 | ||
|
|
||
| import pytest | ||
|
|
||
| from pyiceberg.table import CommitTableResponse, Table | ||
| from pyiceberg.table.update import SetSnapshotRefUpdate, TableUpdate | ||
|
|
||
|
|
||
| def _mock_commit_response(table: Table) -> CommitTableResponse: | ||
| return CommitTableResponse( | ||
| metadata=table.metadata, | ||
| metadata_location="s3://bucket/tbl", | ||
| uuid=uuid4(), | ||
| ) | ||
|
|
||
|
|
||
| def _get_updates(mock_catalog: MagicMock) -> tuple[TableUpdate, ...]: | ||
| args, _ = mock_catalog.commit_table.call_args | ||
| return args[2] | ||
|
|
||
|
|
||
| def test_set_current_snapshot_basic(table_v2: Table) -> None: | ||
| snapshot_one = 3051729675574597004 | ||
|
|
||
| table_v2.catalog = MagicMock() | ||
| table_v2.catalog.commit_table.return_value = _mock_commit_response(table_v2) | ||
|
|
||
| table_v2.manage_snapshots().set_current_snapshot(snapshot_id=snapshot_one).commit() | ||
|
|
||
| table_v2.catalog.commit_table.assert_called_once() | ||
|
|
||
| updates = _get_updates(table_v2.catalog) | ||
| set_ref_updates = [u for u in updates if isinstance(u, SetSnapshotRefUpdate)] | ||
|
|
||
| assert len(set_ref_updates) == 1 | ||
| update = set_ref_updates[0] | ||
| assert update.snapshot_id == snapshot_one | ||
| assert update.ref_name == "main" | ||
| assert update.type == "branch" | ||
|
|
||
|
|
||
| def test_set_current_snapshot_unknown_id(table_v2: Table) -> None: | ||
| invalid_snapshot_id = 1234567890000 | ||
| table_v2.catalog = MagicMock() | ||
|
|
||
| with pytest.raises(ValueError, match="Cannot set current snapshot to unknown snapshot id"): | ||
| table_v2.manage_snapshots().set_current_snapshot(snapshot_id=invalid_snapshot_id).commit() | ||
|
|
||
| table_v2.catalog.commit_table.assert_not_called() | ||
|
|
||
|
|
||
| def test_set_current_snapshot_to_current(table_v2: Table) -> None: | ||
| current_snapshot = table_v2.current_snapshot() | ||
| assert current_snapshot is not None | ||
|
|
||
| table_v2.catalog = MagicMock() | ||
| table_v2.catalog.commit_table.return_value = _mock_commit_response(table_v2) | ||
|
|
||
| table_v2.manage_snapshots().set_current_snapshot(snapshot_id=current_snapshot.snapshot_id).commit() | ||
|
|
||
| table_v2.catalog.commit_table.assert_called_once() | ||
|
|
||
|
|
||
| def test_set_current_snapshot_chained_with_tag(table_v2: Table) -> None: | ||
| snapshot_one = 3051729675574597004 | ||
| table_v2.catalog = MagicMock() | ||
| table_v2.catalog.commit_table.return_value = _mock_commit_response(table_v2) | ||
|
|
||
| (table_v2.manage_snapshots().set_current_snapshot(snapshot_id=snapshot_one).create_tag(snapshot_one, "my-tag").commit()) | ||
|
|
||
| table_v2.catalog.commit_table.assert_called_once() | ||
|
|
||
| updates = _get_updates(table_v2.catalog) | ||
| set_ref_updates = [u for u in updates if isinstance(u, SetSnapshotRefUpdate)] | ||
|
|
||
| assert len(set_ref_updates) == 2 | ||
| assert {u.ref_name for u in set_ref_updates} == {"main", "my-tag"} | ||
|
|
||
|
|
||
| def test_set_current_snapshot_with_extensive_snapshots(table_v2_with_extensive_snapshots: Table) -> None: | ||
| snapshots = table_v2_with_extensive_snapshots.metadata.snapshots | ||
| assert len(snapshots) > 100 | ||
|
|
||
| target_snapshot = snapshots[50].snapshot_id | ||
|
|
||
| table_v2_with_extensive_snapshots.catalog = MagicMock() | ||
| table_v2_with_extensive_snapshots.catalog.commit_table.return_value = _mock_commit_response(table_v2_with_extensive_snapshots) | ||
|
|
||
| table_v2_with_extensive_snapshots.manage_snapshots().set_current_snapshot(snapshot_id=target_snapshot).commit() | ||
|
|
||
| table_v2_with_extensive_snapshots.catalog.commit_table.assert_called_once() | ||
|
|
||
| updates = _get_updates(table_v2_with_extensive_snapshots.catalog) | ||
| set_ref_updates = [u for u in updates if isinstance(u, SetSnapshotRefUpdate)] | ||
|
|
||
| assert len(set_ref_updates) == 1 | ||
| assert set_ref_updates[0].snapshot_id == target_snapshot | ||
|
|
||
|
|
||
| def test_set_current_snapshot_by_ref_name(table_v2: Table) -> None: | ||
| current_snapshot = table_v2.current_snapshot() | ||
| assert current_snapshot is not None | ||
|
|
||
| table_v2.catalog = MagicMock() | ||
| table_v2.catalog.commit_table.return_value = _mock_commit_response(table_v2) | ||
|
|
||
| table_v2.manage_snapshots().set_current_snapshot(ref_name="main").commit() | ||
|
|
||
| updates = _get_updates(table_v2.catalog) | ||
| set_ref_updates = [u for u in updates if isinstance(u, SetSnapshotRefUpdate)] | ||
|
|
||
| assert len(set_ref_updates) == 1 | ||
| assert set_ref_updates[0].snapshot_id == current_snapshot.snapshot_id | ||
| assert set_ref_updates[0].ref_name == "main" | ||
|
|
||
|
|
||
| def test_set_current_snapshot_unknown_ref(table_v2: Table) -> None: | ||
| table_v2.catalog = MagicMock() | ||
|
|
||
| with pytest.raises(ValueError, match="Cannot find matching snapshot ID for ref: nonexistent"): | ||
| table_v2.manage_snapshots().set_current_snapshot(ref_name="nonexistent").commit() | ||
|
|
||
| table_v2.catalog.commit_table.assert_not_called() | ||
|
|
||
|
|
||
| def test_set_current_snapshot_requires_one_argument(table_v2: Table) -> None: | ||
| table_v2.catalog = MagicMock() | ||
|
|
||
| with pytest.raises(ValueError, match="Either snapshot_id or ref_name must be provided, not both"): | ||
| table_v2.manage_snapshots().set_current_snapshot().commit() | ||
|
|
||
| with pytest.raises(ValueError, match="Either snapshot_id or ref_name must be provided, not both"): | ||
| table_v2.manage_snapshots().set_current_snapshot(snapshot_id=123, ref_name="main").commit() | ||
|
|
||
| table_v2.catalog.commit_table.assert_not_called() | ||
|
|
||
|
|
||
| def test_set_current_snapshot_chained_with_create_tag(table_v2: Table) -> None: | ||
| snapshot_one = 3051729675574597004 | ||
| table_v2.catalog = MagicMock() | ||
| table_v2.catalog.commit_table.return_value = _mock_commit_response(table_v2) | ||
|
|
||
| # create a tag and immediately use it to set current snapshot | ||
| ( | ||
| table_v2.manage_snapshots() | ||
| .create_tag(snapshot_id=snapshot_one, tag_name="new-tag") | ||
| .set_current_snapshot(ref_name="new-tag") | ||
| .commit() | ||
| ) | ||
|
|
||
| table_v2.catalog.commit_table.assert_called_once() | ||
|
|
||
| updates = _get_updates(table_v2.catalog) | ||
| set_ref_updates = [u for u in updates if isinstance(u, SetSnapshotRefUpdate)] | ||
|
|
||
| # should have the tag and the main branch update | ||
| assert len(set_ref_updates) == 2 | ||
| assert {u.ref_name for u in set_ref_updates} == {"new-tag", "main"} | ||
|
|
||
| # The main branch should point to the same snapshot as the tag | ||
| main_update = next(u for u in set_ref_updates if u.ref_name == "main") | ||
| assert main_update.snapshot_id == snapshot_one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this! This is a clean and explicit api surface 😄 . We dont need to worry about the state of
_autocommitcc @Fokko WDYT about adding this helper method to the base Transaction class?