Skip to content

[#11094] feat(client-python): add Group authorization management#11109

Merged
jerryshao merged 6 commits into
apache:mainfrom
sunyuhan1998:feature/python-sdk-auth-group
May 22, 2026
Merged

[#11094] feat(client-python): add Group authorization management#11109
jerryshao merged 6 commits into
apache:mainfrom
sunyuhan1998:feature/python-sdk-auth-group

Conversation

@sunyuhan1998
Copy link
Copy Markdown
Contributor

Summary

This PR adds Group authorization management support to the Gravitino Python client SDK, following the pattern established in #11058 (User management). It is split from the original monolithic PR #10783.

Changes

  • API: Group interface with name() and roles() methods
  • DTO: GroupDTO with immutable roles (tuple[str, ...] internally, defensive list copy via roles()); audit null validation in Builder
  • Request/Response: GroupAddRequest, GroupResponse, GroupListResponse, GroupNamesListResponse
  • Error Handler: GroupErrorHandler mapping REST error codes to NoSuchGroupException, GroupAlreadyExistsException, etc.
  • Client: 5 CRUD methods on both GravitinoMetalake and GravitinoClient (add_group, remove_group, get_group, list_groups, list_group_names)
  • Tests: Unit tests (33 cases) and integration tests (6 cases)

Sub-task of

Related issues

Reviewers

@jerryshao

@sunyuhan1998
Copy link
Copy Markdown
Contributor Author

@jerryshao Could you help review this PR? Thanks a lot.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 19, 2026

Code Coverage Report

Overall Project 66.15% +0.01% 🟢
Files changed 74.58% 🟢

Module Coverage
aliyun 1.72% 🔴
api 46.83% 🟢
authorization-common 85.96% 🟢
aws 1.08% 🔴
azure 2.47% 🔴
catalog-common 10.2% 🔴
catalog-fileset 80.02% 🟢
catalog-glue 64.03% 🟢
catalog-hive 79.59% 🟢
catalog-jdbc-clickhouse 80.02% 🟢
catalog-jdbc-common 44.46% 🟢
catalog-jdbc-doris 80.28% 🟢
catalog-jdbc-hologres 54.03% 🟢
catalog-jdbc-mysql 79.23% 🟢
catalog-jdbc-oceanbase 78.38% 🟢
catalog-jdbc-postgresql 82.17% 🟢
catalog-jdbc-starrocks 78.27% 🟢
catalog-kafka 77.01% 🟢
catalog-lakehouse-generic 44.89% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.8% 🟢
catalog-lakehouse-paimon 79.29% 🟢
catalog-model 77.72% 🟢
cli 44.51% 🟢
client-java 77.92% 🟢
common 49.99% 🟢
core 82.2% 🟢
filesystem-hadoop3 76.97% 🟢
flink 0.0% 🔴
flink-common 43.17% 🟢
flink-runtime 0.0% 🔴
gcp 14.12% 🔴
hadoop-common 10.39% 🔴
hive-metastore-common 53.26% 🟢
iceberg-common 55.24% 🟢
iceberg-rest-server 69.69% +0.09% 🟢
idp-basic 94.17% 🟢
integration-test-common 0.0% 🔴
jobs 66.17% 🟢
lance-common 20.9% 🔴
lance-rest-server 62.78% 🟢
lineage 53.02% 🟢
optimizer 82.95% 🟢
optimizer-api 21.95% 🔴
server 85.75% 🟢
server-common 72.83% 🟢
spark 32.79% 🔴
spark-common 39.09% 🔴
trino-connector 34.82% 🔴
Files
Module File Coverage
iceberg-rest-server IcebergMetadataAuthorizationMethodInterceptor.java 74.58% 🟢

@jerryshao jerryshao requested a review from Copilot May 19, 2026 07:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@sunyuhan1998
Copy link
Copy Markdown
Contributor Author

@jerryshao Looks like Copilot failed, could you retrigger it?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Comment thread clients/client-python/gravitino/dto/authorization/group_dto.py
Comment thread clients/client-python/tests/integration/test_group_management.py Outdated
Comment thread clients/client-python/gravitino/dto/requests/group_add_request.py
@sunyuhan1998 sunyuhan1998 force-pushed the feature/python-sdk-auth-group branch from f1427ff to 22317a4 Compare May 20, 2026 12:37
Add Group entity support to the Python SDK, including:
- Group API interface and GroupDTO with immutable roles
- GroupAddRequest, GroupResponse, GroupListResponse, GroupNamesListResponse
- GroupErrorHandler for REST error mapping
- GravitinoMetalake group CRUD methods (add/remove/get/list/list_names)
- GravitinoClient delegate methods
- Unit tests and integration tests
Fix setUpClass to always modify gravitino.conf and restart the server
when running under Gradle (START_EXTERNAL_GRAVITINO=true). Add
tearDownClass to restore the config after tests.
…auth

- Remove test_builder_no_audit_raises to match relaxed GroupDTO builder
- Add unit test for GroupAddRequest
- Add serviceAdmins config in group integration test
@sunyuhan1998 sunyuhan1998 force-pushed the feature/python-sdk-auth-group branch from 22317a4 to ce1b20c Compare May 20, 2026 12:43
@sunyuhan1998
Copy link
Copy Markdown
Contributor Author

@jerryshao The PythonIT failure in the latest run seems to be a CI environment issue — the Gravitino server failed to start (Can't start Gravitino server!), which caused all integration tests to fail at setUpClass. Could you please retrigger the CI? Thanks!

@jerryshao
Copy link
Copy Markdown
Contributor

The IT still fails. Maybe you can download the log here https://github.com/apache/gravitino/actions/runs/26163265995?pr=11109 and investigate it locally.

…config

- Fix test_group_management tearDown to properly restore auth config
- Fix test_user setUp to include serviceAdmins and tearDown to restore config
- Format test_owner with black
@sunyuhan1998
Copy link
Copy Markdown
Contributor Author

@jerryshao The CI has passed now. The root cause of the previous IT failure was that tearDownClass in test_group_management and test_user didn't properly restore the server config — setting serviceAdmins to an empty string caused subsequent tests to fail on server restart. I've fixed both files to correctly reset and restore the authorization config in tearDown. Could you take another look? Thanks!

@jerryshao jerryshao merged commit 67c867c into apache:main May 22, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Subtask] Add Group authorization management to Python client

3 participants