Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement] The role command supports handling multiple values simultaneously. #5985

Closed
Abyss-lord opened this issue Dec 25, 2024 · 1 comment · Fixed by #5988
Closed
Assignees
Labels
improvement Improvements on everything

Comments

@Abyss-lord
Copy link
Contributor

What would you like to be improved?

In GravitinoOptions, the role option supports multiple values, but the handleRoleCommand implementation currently only processes a single role value.
image
image

How should we improve?

Make the ROLE command support multiple values simultaneously, and ensure it behaves in the same way as the TAG command.

@Abyss-lord Abyss-lord added the improvement Improvements on everything label Dec 25, 2024
@Abyss-lord Abyss-lord changed the title [Improvement] The role command supports handling multiple values at the same time [Improvement] The role command supports handling multiple values simultaneously. Dec 25, 2024
@Abyss-lord
Copy link
Contributor Author

@xunliu I would like to work on it.

Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 25, 2024
…ng multiple values

Fix role command that supports handling multiple values, CLI can create and delete multiple roles simultaneously.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 25, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 26, 2024
…ng multiple values

Fix some issue due to reviewer's opinion.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 29, 2024
…ng multiple values (apache#5988)

### What changes were proposed in this pull request?

In `GravitinoOptions`, the role option supports multiple values, but the
handleRoleCommand implementation currently only processes a single role
value. CLI should support create and delete multiple roles
simultaneously.


### Why are the changes needed?

Fix: apache#5985 

### Does this PR introduce _any_ user-facing change?

NO

### How was this patch tested?

local test + UT 

```bash
gcli role create -m demo_metalake --role role1 role2 role3
# role1, role2, role3 created

gcli role delete -m demo_metalake --role role1 role2 role3
# role1, role2, role3 deleted.

gcli role delete -m demo_metalake --role role1 role2 role3 unknown
# role1, role2, role3 deleted, but unknown is not deleted.

gcli role details -m demo_metalake
# Missing --role option.

gcli role details -m demo_metalake --role roleA roleB
# Exception in thread "main" java.lang.IllegalArgumentException: details requires only one role, but multiple are currently passed.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvements on everything
Projects
None yet
1 participant