tengqm commented on code in PR #5641:
URL: https://github.com/apache/gravitino/pull/5641#discussion_r1856696932
##########
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##########
@@ -366,10 +368,11 @@ protected void handleTagCommand() {
String url = getUrl();
FullName name = new FullName(line);
String metalake = name.getMetalakeName();
- String tag = line.getOptionValue(GravitinoOptions.TAG);
+ String[] tags = line.getOptionValues(GravitinoOptions.TAG);
+ tags = tags != null ?
Arrays.stream(tags).distinct().toArray(String[]::new) : null;
Review Comment:
This is insane ...
```suggestion
if (tags != null) {
tags = Arrays.stream(tags).distinct().toArray(String[]::new)
}
```
##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTag.java:
##########
@@ -19,39 +19,51 @@
package org.apache.gravitino.cli.commands;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
import org.apache.gravitino.cli.ErrorMessages;
import org.apache.gravitino.client.GravitinoClient;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.TagAlreadyExistsException;
public class CreateTag extends Command {
protected final String metalake;
- protected final String tag;
+ protected final String[] tags;
protected final String comment;
/**
- * Create a new tag.
+ * Create tags.
*
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
* @param metalake The name of the metalake.
- * @param tag The name of the tag.
+ * @param tags The names of the tags.
* @param comment The comment of the tag.
*/
public CreateTag(
- String url, boolean ignoreVersions, String metalake, String tag, String
comment) {
+ String url, boolean ignoreVersions, String metalake, String[] tags,
String comment) {
super(url, ignoreVersions);
this.metalake = metalake;
- this.tag = tag;
+ this.tags = tags;
Review Comment:
There is no validation rule for tags, right?
Basically, any character is acceptable?
##########
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##########
@@ -378,40 +381,44 @@ protected void handleTagCommand() {
}
} else if (CommandActions.CREATE.equals(command)) {
String comment = line.getOptionValue(GravitinoOptions.COMMENT);
- newCreateTag(url, ignore, metalake, tag, comment).handle();
+ newCreateTags(url, ignore, metalake, tags, comment).handle();
} else if (CommandActions.DELETE.equals(command)) {
boolean force = line.hasOption(GravitinoOptions.FORCE);
- newDeleteTag(url, ignore, force, metalake, tag).handle();
+ newDeleteTag(url, ignore, force, metalake, tags).handle();
} else if (CommandActions.SET.equals(command)) {
String property = line.getOptionValue(GravitinoOptions.PROPERTY);
String value = line.getOptionValue(GravitinoOptions.VALUE);
-
if (property != null && value != null) {
- newSetTagProperty(url, ignore, metalake, tag, property,
value).handle();
+ newSetTagProperty(url, ignore, metalake, getOneTag(tags), property,
value).handle();
} else if (name != null && property == null && value == null) {
- newTagEntity(url, ignore, metalake, name, tag).handle();
+ newTagEntity(url, ignore, metalake, name, tags).handle();
}
} else if (CommandActions.REMOVE.equals(command)) {
String property = line.getOptionValue(GravitinoOptions.PROPERTY);
if (property != null) {
- newRemoveTagProperty(url, ignore, metalake, tag, property).handle();
+ newRemoveTagProperty(url, ignore, metalake, getOneTag(tags),
property).handle();
} else {
- newUntagEntity(url, ignore, metalake, name, tag).handle();
+ newUntagEntity(url, ignore, metalake, name, tags).handle();
}
} else if (CommandActions.PROPERTIES.equals(command)) {
- newListTagProperties(url, ignore, metalake, tag).handle();
+ newListTagProperties(url, ignore, metalake, getOneTag(tags)).handle();
} else if (CommandActions.UPDATE.equals(command)) {
if (line.hasOption(GravitinoOptions.COMMENT)) {
String comment = line.getOptionValue(GravitinoOptions.COMMENT);
- newUpdateTagComment(url, ignore, metalake, tag, comment).handle();
+ newUpdateTagComment(url, ignore, metalake, getOneTag(tags),
comment).handle();
}
if (line.hasOption(GravitinoOptions.RENAME)) {
String newName = line.getOptionValue(GravitinoOptions.RENAME);
- newUpdateTagName(url, ignore, metalake, tag, newName).handle();
+ newUpdateTagName(url, ignore, metalake, getOneTag(tags),
newName).handle();
}
}
}
+ private String getOneTag(String[] tags) {
+ Preconditions.checkArgument(tags.length <= 1,
ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR);
+ return tags[0];
Review Comment:
`tags` can be empty, right?
What happens if tags.length == 0?
##########
docs/cli.md:
##########
@@ -456,10 +456,10 @@ gcli group delete --group new_group
gcli tag details --tag tagA
```
-#### Create a tag
+#### Create tags
```bash
- gcli tag create --tag tagA
+ gcli tag create --tag tagA tagB
Review Comment:
Still, I don't agree to the syntax.
When you are creating a tag, you specify the tags as the operand. I.e
`gcli tag create tagA` or `gcli tag create tagA,tagB`
It is very uncommon to have `--tag` as the argument when you invoke `tag
create`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
