Skip to content

Commit e9c91ac

Browse files
geruhsmaheshwar-pltr
authored andcommitted
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 e1667b6 commit e9c91ac

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

pyiceberg/catalog/rest/__init__.py

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

344344
class NamespaceResponse(IcebergBaseModel):
345345
namespace: Identifier = Field()
346-
properties: Properties = Field()
346+
properties: Properties = Field(default_factory=dict)
347+
348+
@field_validator("properties", mode="before")
349+
@classmethod
350+
def replace_none_with_dict(cls, v: Any) -> Properties:
351+
if v is None:
352+
return {}
353+
return v
347354

348355

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