Skip to content

Commit fa9094b

Browse files
oxFokkokevinjqliu
authored
Allow snapshot-id in assert-ref-snapshot-id requirement to serialize to null in json (#2343)
Closes #2342 # Rationale for this change The OpenAPI spec specifies that `snapshot-id` is required [1], even if it's `null`. The current code omits the field b/c `exclude_none=True` is set for all models that extend `IcebergBaseModel`. Setting `exclude=False` on the field doesn't override the behavior and thus the model needs to control it's own serialization. This is the only model I know of so far that has this "required null" problem so I held back from making another class for this model to extend. # Are these changes tested? Yes # Are there any user-facing changes? Set `snapshot-id: null` when committing tables that don't have a current snapshot-id. [1]: https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L3138-L3155 --------- Co-authored-by: Fokko Driesprong <[email protected]> Co-authored-by: Kevin Liu <[email protected]>
1 parent f805488 commit fa9094b

File tree

2 files changed

+11
-1
lines changed

2 files changed

+11
-1
lines changed

pyiceberg/table/update/__init__.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
from abc import ABC, abstractmethod
2222
from datetime import datetime
2323
from functools import singledispatch
24-
from typing import TYPE_CHECKING, Annotated, Any, Dict, Generic, List, Literal, Optional, Tuple, TypeVar, Union, cast
24+
from typing import TYPE_CHECKING, Annotated, Any, Dict, Generic, List, Literal, Optional, Set, Tuple, TypeVar, Union, cast
2525

2626
from pydantic import Field, field_validator, model_validator
2727

@@ -745,6 +745,13 @@ def validate(self, base_metadata: Optional[TableMetadata]) -> None:
745745
elif self.snapshot_id is not None:
746746
raise CommitFailedException(f"Requirement failed: branch or tag {self.ref} is missing, expected {self.snapshot_id}")
747747

748+
# override the override method, allowing None to serialize to `null` instead of being omitted.
749+
def model_dump_json(
750+
self, exclude_none: bool = False, exclude: Optional[Set[str]] = None, by_alias: bool = True, **kwargs: Any
751+
) -> str:
752+
# `snapshot-id` is required in json response, even if null
753+
return super().model_dump_json(exclude_none=False)
754+
748755

749756
class AssertLastAssignedFieldId(ValidatableTableRequirement):
750757
"""The table's last assigned column id must match the requirement's `last-assigned-field-id`."""

tests/table/test_init.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,9 @@ def test_assert_ref_snapshot_id(table_v2: Table) -> None:
10391039
):
10401040
AssertRefSnapshotId(ref="test", snapshot_id=3055729675574597004).validate(base_metadata)
10411041

1042+
expected_json = '{"type":"assert-ref-snapshot-id","ref":"main","snapshot-id":null}'
1043+
assert AssertRefSnapshotId(ref="main").model_dump_json() == expected_json
1044+
10421045

10431046
def test_assert_last_assigned_field_id(table_v2: Table) -> None:
10441047
base_metadata = table_v2.metadata

0 commit comments

Comments
 (0)