vanphuoc3012 opened a new pull request, #12325:
URL: https://github.com/apache/gravitino/pull/12325
### What changes were proposed in this pull request?
`RemoveCatalogPropertyRequest` in
`clients/client-python/gravitino/dto/requests/catalog_update_request.py` was
serialized without the `property` field, so
`GravitinoClient.alter_catalog(...)` with a
`CatalogChange.remove_property(...)` always failed server-side with:
```
IllegalArgumentException: "property" field is required and cannot be empty
```
The class had two compounding defects vs. its siblings: it was missing the
`@dataclass` decorator, and its field was declared as `property: Optional[str]
= None` while `__init__` set `self._property` (a different attribute). It now
matches the sibling pattern (`RemoveTablePropertyRequest`,
`RemoveSchemaPropertyRequest`):
```python
@dataclass
class RemoveCatalogPropertyRequest(CatalogUpdateRequestBase):
_property: Optional[str] = field(
default=None, metadata=config(field_name="property")
)
```
Tests added:
- `tests/unittests/dto/requests/test_catalog_update_request.py` —
serialize/validate coverage for all four catalog update request types (catalog
update requests previously had no unit-test coverage, unlike
table/tag/view/schema).
- `test_alter_catalog_remove_property` integration test in
`tests/integration/test_catalog.py` — sets then removes a catalog property
end-to-end.
### Why are the changes needed?
Any `alter_catalog` call that needs to remove a property is broken (e.g.
config-driven provisioners reconciling catalog properties).
Fix: #12324
### Does this PR introduce _any_ user-facing change?
No. It fixes existing broken behavior for removing catalog properties via
the Python SDK; no API or property-key change.
### How was this patch tested?
- New unit test `test_remove_catalog_property_request_serialize` was red
before the fix (`{"@type": "removeProperty"}`) and green after (`{"@type":
"removeProperty", "property": "prop1"}`).
- Full `tests/unittests/dto/requests/` suite: 91 passed.
- `ruff format --check` passes on changed files.
- Integration test `test_alter_catalog_remove_property` verifies set+remove
end-to-end (requires a running Gravitino server via `GRAVITINO_HOME`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
