[#5061] Basic user and group CLI #5133
Conversation
# Conflicts: # clients/cli/README.md # clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java # clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
# Conflicts: # clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java # clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
# Conflicts: # clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java # clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java # clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java # clients/cli/src/main/java/org/apache/gravitino/cli/Properties.java # clients/cli/src/test/java/org/apache/gravitino/cli/PropertiesTest.java # docs/cli.md
file is not needed
…SV.xxx File is not needed
fix comments
|
@shaofengshi @jerryshao I've updated the PR with the main branch now that the second CLI PR has been merged - can you please review. |
|
|
||
| // Properties option can have multiple values | ||
| Option properties = | ||
| createArgOption("p", PROPERTIES, "comma separated property name/value pairs"); | ||
| properties.hasArgs(); | ||
| Option.builder("p").longOpt(PROPERTIES).desc("property name/value pairs").hasArgs().build(); |
There was a problem hiding this comment.
"comma separated" is a necessary description for the properties I think, why not keep it?
There was a problem hiding this comment.
The option has been updated, and they no longer have to be comma separated - see the description in cli.md.
There was a problem hiding this comment.
I see; This change is irrelavant with current PR, now mixed with other changes; Next time please use a seperate PR, that will be helpful for review as well as code management (revert, cherry-pick, etc).
clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteGroup.java
Outdated
Show resolved
Hide resolved
clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateGroup.java
Show resolved
Hide resolved
| * @return A map of entries, where each entry represents a key-value pair from the input string. | ||
| */ | ||
| public Map<String, String> parse(String input) { | ||
| public Map<String, String> parse(String[] inputs) { |
There was a problem hiding this comment.
Is this change related with the "user and group" CLI? seems it has already been handled in previous PR, please double check whether this is a code merge issue.
There was a problem hiding this comment.
It not a code merge issue - the way properties have been handled have been improved
|
@shaofengshi all comments addressed this is ready for review again. |
What changes were proposed in this pull request?
Added basic user and group commands.
Why are the changes needed?
For the CLI to support users and groups.
Fix: #5061
Does this PR introduce any user-facing change?
No, but extends CLI support.
How was this patch tested?
Tested locally.