Skip to content

Commit 4ab9dc9

Browse files
committed
Enforce positive retention values on SetSnapshotRefUpdate
SnapshotRef requires min-snapshots-to-keep, max-snapshot-age-ms and max-ref-age-ms to be greater than 0, but SetSnapshotRefUpdate did not, so an invalid value only failed when the ref was built at apply time. Java validates these in SnapshotRef.Builder when they are set.
1 parent 8c67227 commit 4ab9dc9

2 files changed

Lines changed: 14 additions & 3 deletions

File tree

‎pyiceberg/table/update/__init__.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ class SetSnapshotRefUpdate(IcebergBaseModel):
138138
ref_name: str = Field(alias="ref-name")
139139
type: Literal[SnapshotRefType.TAG, SnapshotRefType.BRANCH]
140140
snapshot_id: int = Field(alias="snapshot-id")
141-
max_ref_age_ms: Annotated[int | None, Field(alias="max-ref-age-ms", default=None)]
142-
max_snapshot_age_ms: Annotated[int | None, Field(alias="max-snapshot-age-ms", default=None)]
143-
min_snapshots_to_keep: Annotated[int | None, Field(alias="min-snapshots-to-keep", default=None)]
141+
max_ref_age_ms: Annotated[int | None, Field(alias="max-ref-age-ms", default=None, gt=0)]
142+
max_snapshot_age_ms: Annotated[int | None, Field(alias="max-snapshot-age-ms", default=None, gt=0)]
143+
min_snapshots_to_keep: Annotated[int | None, Field(alias="min-snapshots-to-keep", default=None, gt=0)]
144144

145145

146146
class RemoveSnapshotsUpdate(IcebergBaseModel):

‎tests/table/test_manage_snapshots.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from uuid import uuid4
1919

2020
import pytest
21+
from pydantic import ValidationError as PydanticValidationError
2122

2223
from pyiceberg.table import CommitTableResponse, Table
2324
from pyiceberg.table.refs import SnapshotRef, SnapshotRefType
@@ -391,3 +392,13 @@ def test_fast_forward_branch_unknown_snapshot(table_v2: Table) -> None:
391392
table_v2.manage_snapshots().fast_forward_branch(from_branch="main", to_ref="dangling")
392393

393394
table_v2.catalog.commit_table.assert_not_called()
395+
396+
397+
def test_create_branch_invalid_retention(table_v2: Table) -> None:
398+
table_v2.catalog = MagicMock()
399+
400+
# rejected when passed, not when the ref is built
401+
with pytest.raises(PydanticValidationError, match="min_snapshots_to_keep"):
402+
table_v2.manage_snapshots().create_branch(snapshot_id=PARENT_SNAPSHOT_ID, branch_name="b", min_snapshots_to_keep=0)
403+
404+
table_v2.catalog.commit_table.assert_not_called()

0 commit comments

Comments
 (0)