[#10782] feat(client-python): add User authorization management#11058
Merged
jerryshao merged 4 commits intoMay 14, 2026
Conversation
Add User management capabilities to the Python client, supporting the same set of User operations already exposed by the Java client and REST API layer. This is part of a larger split of apache#10783, which introduced the full authorization model in a single PR. To make review easier, the change is being broken down by entity; subsequent PRs will add Group, Role, Privilege, SecurableObject and grant/revoke operations. Changes - Add User interface and UserDTO (Builder-based, immutable) - Add UserAddRequest / UserResponse / UserListResponse / UserNamesListResponse DTOs, plus a shared RemoveResponse - Add add_user / get_user / remove_user / list_users / list_user_names methods to GravitinoMetalake - Add UserErrorHandler to map User-related REST errors to NoSuchUserException, UserAlreadyExistsException, etc. - Add 28 unit tests covering DTO serialization, response parsing, GravitinoMetalake HTTP interactions (URL, method, body, error propagation) and the error-handler mapping table
sunyuhan1998
force-pushed
the
feature/python-sdk-auth-user
branch
from
c7e515e to
687f945
Compare
Code Coverage Report
Files
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Python client support for authorization User management, including DTOs, REST request/response types, client methods, and error handling to begin parity with the Java SDK / REST API.
Changes:
- Introduces
UserAPI interface andUserDTO(+ request/response DTOs) for serialization/deserialization. - Adds
GravitinoMetalakeuser-management methods: add/get/remove/list users and list user names. - Adds user-specific REST error handler and expands unit tests for DTOs, client calls, and error mapping.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| clients/client-python/gravitino/api/authorization/init.py | Exposes User in authorization API exports. |
| clients/client-python/gravitino/api/authorization/user.py | Defines User interface with name() and roles(). |
| clients/client-python/gravitino/client/gravitino_metalake.py | Adds public metalake user-management methods and REST paths. |
| clients/client-python/gravitino/dto/authorization/user_dto.py | Adds UserDTO with builder, JSON mapping, and equality/hash. |
| clients/client-python/gravitino/dto/requests/user_add_request.py | Adds request DTO for creating a user. |
| clients/client-python/gravitino/dto/responses/remove_response.py | Adds shared boolean “removed” response type. |
| clients/client-python/gravitino/dto/responses/user_response.py | Adds user response types (single, list, names list). |
| clients/client-python/gravitino/exceptions/base.py | Adds NoSuchUserException and UserAlreadyExistsException. |
| clients/client-python/gravitino/exceptions/handlers/user_error_handler.py | Adds error-code-to-exception mapping for user operations. |
| clients/client-python/tests/unittests/client/test_metalake_user_operations.py | Adds mock-based unit tests for new metalake methods. |
| clients/client-python/tests/unittests/dto/responses/test_user_response.py | Adds unit tests for new user response DTOs. |
| clients/client-python/tests/unittests/dto/test_user_dto.py | Adds unit tests for UserDTO builder/serde/equality/hash. |
| clients/client-python/tests/unittests/test_error_handler.py | Extends error-handler tests to cover UserErrorHandler. |
jerryshao
reviewed
May 13, 2026
…edback - Add User delegate methods to GravitinoClient (add_user, get_user, remove_user, list_users, list_user_names) — matching the Java SDK pattern and addressing jerryshao's review comment - Make UserDTO._roles immutable (tuple internally, list externally) to preserve hash/equality semantics (Copilot feedback) - Add Optional[UserDTO] type hint to UserResponse._user field (Copilot feedback) - Expand docstrings on GravitinoMetalake User methods with Args/Returns/Raises, keeping `user` parameter name to match Java SDK - Add 7 new tests: UserDTO immutability, GravitinoClient delegation
jerryshao
previously approved these changes
May 14, 2026
Contributor
|
Can you fix some style issues here? |
Fix CI pylint failure in TestGravitinoClientUserDelegates: - Use patch.object(GravitinoClient, "get_metalake") instead of accessing _metalake directly (protected-access W0212) - Remove unused variable and import warnings (W0612, W0611)
Contributor
Author
No problem. I just noticed the CI failure too, and I’ve already pushed a commit to fix it. |
jerryshao
approved these changes
May 14, 2026
Contributor
|
Don't forget to add integration tests for this PR and the previous one. |
Contributor
Author
No problem. I'll submit the relevant PR for integration testing shortly. |
This was referenced May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This PR is split from #10783 to make review easier. It focuses on the User part of the authorization model; Group, Role, Privilege, SecurableObject and grant/revoke operations will follow in subsequent PRs.
Changes in this PR:
API layer (
gravitino/api/authorization/)User— user interface withname()/roles()accessorsDTO layer (
gravitino/dto/authorization/)UserDTO— immutable DTO with builder pattern, serializationand audit info
Request / Response DTOs (
gravitino/dto/)UserAddRequestUserResponse/UserListResponse/UserNamesListResponseRemoveResponse(shared boolean-remove response used acrossauthorization operations)
Client layer (
gravitino/client/gravitino_metalake.py)GravitinoMetalake:add_user/get_user/remove_user/list_users/list_user_namesException handling (
gravitino/exceptions/)NoSuchUserException,UserAlreadyExistsExceptionUserErrorHandlermapping REST error codes to the aboveWhy are the changes needed?
The Python SDK currently has no authorization management capabilities, while the Java SDK and REST API have full support. This PR starts bringing the Python SDK to feature parity with the Java SDK, beginning with User management as the most foundational entity.
Fix: #11098
Does this PR introduce any user-facing change?
Yes. New public API:
GravitinoMetalake.add_user(name)/get_user(name)/remove_user(name)/list_users()/list_user_names()NoSuchUserException,UserAlreadyExistsExceptionNo existing APIs or behaviors are changed.
How was this patch tested?
28 new unit tests (all passing,
ruff+pylint):(
tests/unittests/dto/test_user_dto.py,tests/unittests/dto/responses/test_user_response.py) — cover builder, serialization / deserialization roundtrip, equality(
tests/unittests/client/test_metalake_user_operations.py) — cover URL, HTTP method, request body, response parsing, and error propagation for all fiveGravitinoMetalakemethods(added to
tests/unittests/test_error_handler.py) — verify the full error code mapping table forUserErrorHandlerIntegration tests will be added in the final PR of the split together with grant/revoke operations, where end-to-end multi-entity behavior needs to be validated against a real server.
cc @jerryshao — could you help review this when you have time? Thanks!