Skip to content

Commit 0615eb4

Browse files
authored
fix: Handle optional properties in load namespace properties response (#3169)
Closes #3167 # Rationale for this change The rest spec model `GetNamespaceResponse` defines the `properties` field as optional, and nullable. Also, following the description if the rest catalog doesn't support ns properties they should return null. Link: https://github.com/apache/iceberg/blob/0a73da119ff38ee3a98f248b42180caa51001cec/open-api/rest-catalog-open-api.yaml#L4146-L4163 ```yaml GetNamespaceResponse: type: object required: - namespace properties: namespace: $ref: '#/components/schemas/Namespace' properties: type: object description: Properties stored on the namespace, if supported by the server. If the server does not support namespace properties, it should return null for this field. If namespace properties are supported, but none are set, it should return an empty object. additionalProperties: type: string example: { "owner": "Ralph", 'transient_lastDdlTime': '1452120468' } default: { } nullable: true ``` Looks like the pydantic models raise a `ValidationError` in the optional/null cases as seen in the issue above. So this PR adds a fix to handle these cases. ## Are these changes tested? Yes, added tests and tested with s3tables api ## Are there any user-facing changes? Not really
1 parent 1a54e9c commit 0615eb4

File tree

2 files changed

+41
-1
lines changed

2 files changed

+41
-1
lines changed

pyiceberg/catalog/rest/__init__.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,14 @@ class ListNamespaceResponse(IcebergBaseModel):
337337

338338
class NamespaceResponse(IcebergBaseModel):
339339
namespace: Identifier = Field()
340-
properties: Properties = Field()
340+
properties: Properties = Field(default_factory=dict)
341+
342+
@field_validator("properties", mode="before")
343+
@classmethod
344+
def replace_none_with_dict(cls, v: Any) -> Properties:
345+
if v is None:
346+
return {}
347+
return v
341348

342349

343350
class UpdateNamespacePropertiesResponse(IcebergBaseModel):

tests/catalog/test_rest.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,39 @@ def test_load_namespace_properties_200(rest_mock: Mocker) -> None:
922922
assert RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).load_namespace_properties(namespace) == {"prop": "yes"}
923923

924924

925+
def test_load_namespace_properties_200_without_properties(rest_mock: Mocker) -> None:
926+
namespace = "leden"
927+
rest_mock.get(
928+
f"{TEST_URI}v1/namespaces/{namespace}",
929+
json={"namespace": ["leden"]},
930+
status_code=200,
931+
request_headers=TEST_HEADERS,
932+
)
933+
assert RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).load_namespace_properties(namespace) == {}
934+
935+
936+
def test_load_namespace_properties_200_with_null_properties(rest_mock: Mocker) -> None:
937+
namespace = "leden"
938+
rest_mock.get(
939+
f"{TEST_URI}v1/namespaces/{namespace}",
940+
json={"namespace": ["leden"], "properties": None},
941+
status_code=200,
942+
request_headers=TEST_HEADERS,
943+
)
944+
assert RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).load_namespace_properties(namespace) == {}
945+
946+
947+
def test_load_namespace_properties_200_with_empty_properties(rest_mock: Mocker) -> None:
948+
namespace = "leden"
949+
rest_mock.get(
950+
f"{TEST_URI}v1/namespaces/{namespace}",
951+
json={"namespace": ["leden"], "properties": {}},
952+
status_code=200,
953+
request_headers=TEST_HEADERS,
954+
)
955+
assert RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).load_namespace_properties(namespace) == {}
956+
957+
925958
def test_load_namespace_properties_404(rest_mock: Mocker) -> None:
926959
namespace = "leden"
927960
rest_mock.get(

0 commit comments

Comments
 (0)