[#11095] feat(client-python): add Role CRUD and authorization data structures#11210
Merged
jerryshao merged 3 commits intoMay 27, 2026
Merged
Conversation
…ata structures - Add PrivilegeDTO, SecurableObjectDTO, RoleDTO data transfer objects - Add Role CRUD operations: create_role, get_role, delete_role, list_role_names - Fix GenericPrivilege.__eq__ for PrivilegeDTO compatibility - Add DTOConverters for privilege and securable object conversion - Add unit tests and integration tests for Role CRUD
4 tasks
Contributor
Author
|
Hi @jerryshao , I've split the CRUD part of role into a separate PR, do you think this is appropriate? |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds Role authorization management to the Python client SDK by introducing Role/Privilege/SecurableObject DTOs plus create/get/delete/list role APIs on GravitinoMetalake and GravitinoClient, along with unit + integration test coverage.
Changes:
- Added authorization DTOs:
PrivilegeDTO,SecurableObjectDTO,RoleDTO(JSON serialization + converters). - Added Role CRUD APIs on
GravitinoMetalakeand delegated equivalents onGravitinoClient. - Added new request/response DTOs and expanded unit/integration tests + REST error handler coverage.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| clients/client-python/gravitino/api/authorization/privileges.py | Adjusts GenericPrivilege.__eq__ to compare against any Privilege implementation. |
| clients/client-python/gravitino/client/dto_converters.py | Adds converters to/from PrivilegeDTO and SecurableObjectDTO. |
| clients/client-python/gravitino/client/gravitino_client.py | Adds Role CRUD delegate methods on GravitinoClient. |
| clients/client-python/gravitino/client/gravitino_metalake.py | Implements Role CRUD operations against REST endpoints on GravitinoMetalake. |
| clients/client-python/gravitino/dto/authorization/init.py | Exports new authorization DTOs from the package. |
| clients/client-python/gravitino/dto/authorization/privilege_dto.py | Introduces PrivilegeDTO with JSON serialization and equality/hash. |
| clients/client-python/gravitino/dto/authorization/role_dto.py | Introduces RoleDTO with builder, JSON serialization, equality/hash. |
| clients/client-python/gravitino/dto/authorization/securable_object_dto.py | Introduces SecurableObjectDTO with JSON serialization and derived name/parent. |
| clients/client-python/gravitino/dto/requests/role_create_request.py | Adds RoleCreateRequest for role creation payloads. |
| clients/client-python/gravitino/dto/responses/role_response.py | Adds RoleResponse and RoleNamesListResponse response DTOs. |
| clients/client-python/tests/integration/test_role_management.py | Adds live-server integration tests for role CRUD. |
| clients/client-python/tests/unittests/client/test_metalake_role_operations.py | Adds mock-based unit tests for role endpoints + client delegation. |
| clients/client-python/tests/unittests/dto/responses/test_role_response.py | Adds unit tests for role response DTO validation + JSON roundtrip. |
| clients/client-python/tests/unittests/dto/test_privilege_dto.py | Adds unit tests for PrivilegeDTO. |
| clients/client-python/tests/unittests/dto/test_role_dto.py | Adds unit tests for RoleDTO. |
| clients/client-python/tests/unittests/dto/test_securable_object_dto.py | Adds unit tests for SecurableObjectDTO. |
| clients/client-python/tests/unittests/test_error_handler.py | Extends error-handler tests to cover role error mapping. |
…to concrete privilege PrivilegeDTO.can_bind_to now delegates to the concrete Privilege subclass via Privileges.allow/deny, matching the Java DTO behavior.
Contributor
Author
|
Hi! @jerryshao , I have addressed all the feedback from Copilot. Please help review it again when you have time. Thank you! |
jerryshao
reviewed
May 26, 2026
jerryshao
reviewed
May 26, 2026
… methods Remove from_privilege_dto and from_securable_object_dto which have no callers in this PR. They can be added in the follow-up Grant/Revoke PR.
Contributor
Author
|
@jerryshao , all known issues have been resolved, please help review it again, thank you! |
jerryshao
approved these changes
May 27, 2026
jerryshao
pushed a commit
that referenced
this pull request
Jun 2, 2026
…ns (#11274) ### What changes were proposed in this pull request? Add Grant/Revoke authorization operations to the Python client SDK: - **PermissionErrorHandler**: Error handler for grant/revoke REST API calls - **Grant/Revoke roles**: `grant_roles_to_user`, `revoke_roles_from_user`, `grant_roles_to_group`, `revoke_roles_from_group` - **Grant/Revoke privileges**: `grant_privileges_to_role`, `revoke_privileges_from_role` - **Input validation**: All grant/revoke methods validate parameters with `Precondition.check_string_not_empty` ### Why are the changes needed? This completes the Role authorization piece of issue #10782, building on top of PR #11210 (Role CRUD) which is already merged. Fix: #11096 ### Does this PR introduce _any_ user-facing change? Yes — new public APIs on `GravitinoClient` and `GravitinoMetalake`: - `grant_roles_to_user(role_names, user_name)` - `revoke_roles_from_user(role_names, user_name)` - `grant_roles_to_group(role_names, group_name)` - `revoke_roles_from_group(role_names, group_name)` - `grant_privileges_to_role(role_name, securable_object, privileges)` - `revoke_privileges_from_role(role_name, securable_object, privileges)` ### How was this patch tested? - Unit tests: 6 grant/revoke mock tests, 6 client delegate tests, 13 permission error handler assertions - Integration tests: 3 tests against a live Gravitino server (grant/revoke roles to user, grant/revoke roles to group, grant/revoke privileges to role) - Linting: `ruff check` clean, `pylint` 10/10 --------- Co-authored-by: Sun Yuhan <sunyuhan1998@users.noreply.github.com>
danhuawang
pushed a commit
to danhuawang/gravitino
that referenced
this pull request
Jun 8, 2026
…ata structures (apache#11210) ### What changes were proposed in this pull request? Add Role CRUD operations and authorization data structures to the Python client SDK: - **DTOs**: `PrivilegeDTO`, `SecurableObjectDTO`, `RoleDTO` with JSON serialization support - **Role CRUD**: `create_role`, `get_role`, `delete_role`, `list_role_names` on `GravitinoMetalake` and `GravitinoClient` - **DTOConverters**: `to/from_privilege_dto` and `to/from_securable_object_dto` conversion methods - **Fix**: `GenericPrivilege.__eq__` now checks `isinstance(value, Privilege)` for `PrivilegeDTO` compatibility - **Tests**: Unit tests (DTO, response, client delegate, error handler) and integration tests against a live server ### Why are the changes needed? This is the Role authorization piece of issue apache#10782. User (apache#11058) and Group (apache#11094) management are already merged. Role CRUD is a prerequisite for Grant/Revoke operations, which will follow in a separate PR. Fix: apache#11095 ### Does this PR introduce _any_ user-facing change? Yes — new public APIs on `GravitinoClient` and `GravitinoMetalake`: - `create_role(role_name, properties=None, securable_objects=None)` - `get_role(role_name)` - `delete_role(role_name) -> bool` - `list_role_names() -> list[str]` ### How was this patch tested? - Unit tests: all passed - Integration tests: 4 passed against a live Gravitino server with authorization enabled - Linting: `ruff check` clean, `pylint` 10/10 --------- Co-authored-by: Sun Yuhan <sunyuhan1998@users.noreply.github.com>
danhuawang
pushed a commit
to danhuawang/gravitino
that referenced
this pull request
Jun 8, 2026
…erations (apache#11274) ### What changes were proposed in this pull request? Add Grant/Revoke authorization operations to the Python client SDK: - **PermissionErrorHandler**: Error handler for grant/revoke REST API calls - **Grant/Revoke roles**: `grant_roles_to_user`, `revoke_roles_from_user`, `grant_roles_to_group`, `revoke_roles_from_group` - **Grant/Revoke privileges**: `grant_privileges_to_role`, `revoke_privileges_from_role` - **Input validation**: All grant/revoke methods validate parameters with `Precondition.check_string_not_empty` ### Why are the changes needed? This completes the Role authorization piece of issue apache#10782, building on top of PR apache#11210 (Role CRUD) which is already merged. Fix: apache#11096 ### Does this PR introduce _any_ user-facing change? Yes — new public APIs on `GravitinoClient` and `GravitinoMetalake`: - `grant_roles_to_user(role_names, user_name)` - `revoke_roles_from_user(role_names, user_name)` - `grant_roles_to_group(role_names, group_name)` - `revoke_roles_from_group(role_names, group_name)` - `grant_privileges_to_role(role_name, securable_object, privileges)` - `revoke_privileges_from_role(role_name, securable_object, privileges)` ### How was this patch tested? - Unit tests: 6 grant/revoke mock tests, 6 client delegate tests, 13 permission error handler assertions - Integration tests: 3 tests against a live Gravitino server (grant/revoke roles to user, grant/revoke roles to group, grant/revoke privileges to role) - Linting: `ruff check` clean, `pylint` 10/10 --------- Co-authored-by: Sun Yuhan <sunyuhan1998@users.noreply.github.com>
danhuawang
pushed a commit
to danhuawang/gravitino
that referenced
this pull request
Jun 9, 2026
…ata structures (apache#11210) ### What changes were proposed in this pull request? Add Role CRUD operations and authorization data structures to the Python client SDK: - **DTOs**: `PrivilegeDTO`, `SecurableObjectDTO`, `RoleDTO` with JSON serialization support - **Role CRUD**: `create_role`, `get_role`, `delete_role`, `list_role_names` on `GravitinoMetalake` and `GravitinoClient` - **DTOConverters**: `to/from_privilege_dto` and `to/from_securable_object_dto` conversion methods - **Fix**: `GenericPrivilege.__eq__` now checks `isinstance(value, Privilege)` for `PrivilegeDTO` compatibility - **Tests**: Unit tests (DTO, response, client delegate, error handler) and integration tests against a live server ### Why are the changes needed? This is the Role authorization piece of issue apache#10782. User (apache#11058) and Group (apache#11094) management are already merged. Role CRUD is a prerequisite for Grant/Revoke operations, which will follow in a separate PR. Fix: apache#11095 ### Does this PR introduce _any_ user-facing change? Yes — new public APIs on `GravitinoClient` and `GravitinoMetalake`: - `create_role(role_name, properties=None, securable_objects=None)` - `get_role(role_name)` - `delete_role(role_name) -> bool` - `list_role_names() -> list[str]` ### How was this patch tested? - Unit tests: all passed - Integration tests: 4 passed against a live Gravitino server with authorization enabled - Linting: `ruff check` clean, `pylint` 10/10 --------- Co-authored-by: Sun Yuhan <sunyuhan1998@users.noreply.github.com>
danhuawang
pushed a commit
to danhuawang/gravitino
that referenced
this pull request
Jun 9, 2026
…erations (apache#11274) ### What changes were proposed in this pull request? Add Grant/Revoke authorization operations to the Python client SDK: - **PermissionErrorHandler**: Error handler for grant/revoke REST API calls - **Grant/Revoke roles**: `grant_roles_to_user`, `revoke_roles_from_user`, `grant_roles_to_group`, `revoke_roles_from_group` - **Grant/Revoke privileges**: `grant_privileges_to_role`, `revoke_privileges_from_role` - **Input validation**: All grant/revoke methods validate parameters with `Precondition.check_string_not_empty` ### Why are the changes needed? This completes the Role authorization piece of issue apache#10782, building on top of PR apache#11210 (Role CRUD) which is already merged. Fix: apache#11096 ### Does this PR introduce _any_ user-facing change? Yes — new public APIs on `GravitinoClient` and `GravitinoMetalake`: - `grant_roles_to_user(role_names, user_name)` - `revoke_roles_from_user(role_names, user_name)` - `grant_roles_to_group(role_names, group_name)` - `revoke_roles_from_group(role_names, group_name)` - `grant_privileges_to_role(role_name, securable_object, privileges)` - `revoke_privileges_from_role(role_name, securable_object, privileges)` ### How was this patch tested? - Unit tests: 6 grant/revoke mock tests, 6 client delegate tests, 13 permission error handler assertions - Integration tests: 3 tests against a live Gravitino server (grant/revoke roles to user, grant/revoke roles to group, grant/revoke privileges to role) - Linting: `ruff check` clean, `pylint` 10/10 --------- Co-authored-by: Sun Yuhan <sunyuhan1998@users.noreply.github.com>
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?
Add Role CRUD operations and authorization data structures to the Python client SDK:
PrivilegeDTO,SecurableObjectDTO,RoleDTOwith JSON serialization supportcreate_role,get_role,delete_role,list_role_namesonGravitinoMetalakeandGravitinoClientto/from_privilege_dtoandto/from_securable_object_dtoconversion methodsGenericPrivilege.__eq__now checksisinstance(value, Privilege)forPrivilegeDTOcompatibilityWhy are the changes needed?
This is the Role authorization piece of issue #10782. User (#11058) and Group (#11094) management are already merged. Role CRUD is a prerequisite for Grant/Revoke operations, which will follow in a separate PR.
Fix: #11095
Does this PR introduce any user-facing change?
Yes — new public APIs on
GravitinoClientandGravitinoMetalake:create_role(role_name, properties=None, securable_objects=None)get_role(role_name)delete_role(role_name) -> boollist_role_names() -> list[str]How was this patch tested?
ruff checkclean,pylint10/10