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

[#5831] fix(CLI): Fix CLi gives unexpected output when input tag set command #5897

Merged
merged 48 commits into from
Dec 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
ee7f792
[#5831] fix(CLI): Fix CLi gives unexpected output when setting a tag
Abyss-lord Dec 18, 2024
e9d6f0c
[#5831] fix(CLI): Fix CLi gives unexpected output when setting a tag
Abyss-lord Dec 18, 2024
e7f16d3
[#5831] fix(CLI): Fix CLi gives unexpected output when setting a tag
Abyss-lord Dec 18, 2024
d435355
[#5831] fix(CLI): Fix CLi gives unexpected output when setting a tag
Abyss-lord Dec 19, 2024
863e247
[#5162] Add grant and revoke privileges to the Gravitino CLI. (#5783)
justinmclean Dec 19, 2024
d5d5a9d
[#5896] Exit with -1 when an error occurs in the Gravitino CLI (#5903)
justinmclean Dec 19, 2024
f850e4a
[#5892] fix(auth): Fix to grant privilege for the metalake (#5919)
jerqi Dec 19, 2024
1d16bda
[#5916] improve(auth): Remove AuthorizationPlugin single instance imp…
xunliu Dec 20, 2024
5e4a2ad
[Minor] fix comments to have correct entity (#5931)
justinmclean Dec 20, 2024
c2ede69
[#5828] improvement(CLI): Slightly misleading error message when user…
Abyss-lord Dec 20, 2024
52a299d
[#5829] improvement(CLI): Slightly misleading error message when grou…
Abyss-lord Dec 20, 2024
cb30467
[#5934] fix(auth): Avoid other catalogs' privileges are pushed down (…
jerqi Dec 20, 2024
f933f38
[#5894] feat(iceberg): support Azure account key credential (#5938)
orenccl Dec 22, 2024
ef2f92d
[#5756] Bug Fix : Warehouse parameter systematically required (#5923)
fsalhi2 Dec 23, 2024
b516b32
[#5794] feat(core): Add ModelOperationDispatcher logic (#5908)
jerryshao Dec 23, 2024
b2485bb
[#5911] fix(docs): Fix the wrong possible values. (#5922)
cool9850311 Dec 23, 2024
663f25e
[#5827] improvement(CLI): Fix CLI throws an obscure error when Delete…
Abyss-lord Dec 23, 2024
4cce13c
[#5926] fix(CLI): Fix Missleading error message in Gravitino CLI when…
Abyss-lord Dec 23, 2024
6248395
[#5947] fix(auth): It will throw error if we enable authorization and…
jerqi Dec 23, 2024
dfd490a
[#5928] fix(CLI): Fix columns details command produces no output (#5945)
Abyss-lord Dec 23, 2024
77bd191
[#5929] improvement(CLI): fix cli details command produce no output (…
Abyss-lord Dec 23, 2024
727ef88
[MINOR] Remove add-to-project Github Action (#5951)
jerryshao Dec 23, 2024
df97ea9
[#5924] improvement(CLI): fix cli list command produce no outputs (#5…
Abyss-lord Dec 23, 2024
24f8c45
[#5826] fix(CLI): Fix Dropping a metalake via the Gravitino CLI gives…
Abyss-lord Dec 23, 2024
b34b0c8
[#5927] improvement(CLI): fix cli get multiple "Malformed entity name…
Abyss-lord Dec 23, 2024
a4b7c57
[#5895] support ADLSToken/AzureAccountKey credential for python clien…
orenccl Dec 24, 2024
a4f6afb
[#5969] fix(Docker): Failed to create Docker network by concurrent ex…
xunliu Dec 24, 2024
3d6f5f2
[#5661] feat(auth): Add JDBC authorization plugin interface (#5904)
jerqi Dec 24, 2024
f6806bb
[#5954] feat(iceberg): Supports ADLS for Iceberg catalog and spark co…
FANNG1 Dec 24, 2024
798d5e7
Minor (docs): Update how-to-use-the-playground.md document (#5955)
unknowntpo Dec 24, 2024
13cd1a9
[#5775] feat(auth): Chain authorization plugin framework (#5786)
xunliu Dec 25, 2024
fc7a6b3
[#4714] feat(paimon-spark-connector): Add tests for partitionManageme…
caican00 Dec 25, 2024
525f8aa
[#5861] improvement(CLI): Refactor the validation logic in the handle…
Abyss-lord Dec 25, 2024
e1acf63
[#5930] improvement(CLI): improve unknown tag output. (#5978)
Abyss-lord Dec 26, 2024
06af45c
[#5620] feat(fileset): Support credential vending for fileset catalog…
FANNG1 Dec 26, 2024
6aa714d
[#5817] core(feat): Add server-side REST APIs for model management (#…
jerryshao Dec 26, 2024
132b468
[#5993] refactor: Move the JdbcAuthorizationPlugin to authorization-c…
jerqi Dec 26, 2024
350d788
[#5993][FOLLOWUP] Make some methods public (#6002)
jerqi Dec 26, 2024
af433fd
[#5987] improvement: Add more configurations for the config API (#5999)
jerqi Dec 26, 2024
b3475db
[#4398] feat(core): support credential cache for Gravitino server (#5…
FANNG1 Dec 27, 2024
8db9b8a
[#5585] improvement(bundles): Refactor bundle jars and provide core j…
yuqi1129 Dec 27, 2024
5357dc4
[#5989] The credential type of ADLSTokenCredentialProvider is not equ…
FANNG1 Dec 28, 2024
69e71a3
[#5985] improvement(CLI): Fix role command that supports handling mul…
Abyss-lord Dec 28, 2024
2e20332
[#6014] refactor: CLI output methods for no data hints (#6015)
waukin Dec 28, 2024
cfe6691
[#5831] fix(CLI): Fix CLi gives unexpected output when setting a tag
Abyss-lord Dec 29, 2024
79ac312
[#5831] fix(CLI): Fix CLi gives unexpected output when setting a tag
Abyss-lord Dec 29, 2024
d0f79d8
Merge branch 'main' into fix-tag-command
Abyss-lord Dec 29, 2024
d4ba7cc
[#5831] fix(CLI): Fix CLi gives unexpected output when setting a tag
Abyss-lord Dec 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,15 @@ public String getName() {
return null;
}

/**
* Are there any names that can be retrieved?
*
* @return True if the name exists, or false if it does not.
*/
public Boolean hasName() {
return line.hasOption(GravitinoOptions.NAME);
}

/**
* Helper method to retrieve a specific part of the full name based on the position of the part.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,14 @@ protected void handleTagCommand() {
if (propertySet != null && valueSet != null) {
newSetTagProperty(url, ignore, metalake, getOneTag(tags), propertySet, valueSet).handle();
} else if (propertySet == null && valueSet == null) {
if (!name.hasName()) {
System.err.println(ErrorMessages.MISSING_NAME);
Main.exit(-1);
}
newTagEntity(url, ignore, metalake, name, tags).handle();
} else {
System.err.println("The set command only supports tag properties or attaching tags.");
Main.exit(-1);
Abyss-lord marked this conversation as resolved.
Show resolved Hide resolved
}
break;

Expand All @@ -681,6 +688,10 @@ protected void handleTagCommand() {
if (propertyRemove != null) {
newRemoveTagProperty(url, ignore, metalake, getOneTag(tags), propertyRemove).handle();
} else {
if (!name.hasName()) {
System.err.println(ErrorMessages.MISSING_NAME);
Main.exit(-1);
}
newUntagEntity(url, ignore, metalake, name, tags).handle();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.gravitino.cli.commands;

import com.google.common.base.Joiner;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Schema;
Expand All @@ -32,6 +33,7 @@
import org.apache.gravitino.rel.Table;

public class UntagEntity extends Command {
public static final Joiner COMMA_JOINER = Joiner.on(", ").skipNulls();
protected final String metalake;
protected final FullName name;
protected final String[] tags;
Expand Down Expand Up @@ -91,7 +93,7 @@ public void handle() {
} catch (NoSuchCatalogException err) {
exitWithError(ErrorMessages.UNKNOWN_CATALOG);
} catch (NoSuchSchemaException err) {
exitWithError(ErrorMessages.UNKNOWN_TABLE);
exitWithError(ErrorMessages.UNKNOWN_SCHEMA);
} catch (NoSuchTableException err) {
exitWithError(ErrorMessages.UNKNOWN_TABLE);
} catch (Exception exp) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,21 @@ public void missingArgs() throws Exception {
assertNull(namePart);
}

@Test
public void hasPartName() throws ParseException {
String[] argsWithoutName = {"catalog", "details", "--metalake", "metalake"};
CommandLine commandLineWithoutName = new DefaultParser().parse(options, argsWithoutName);
FullName fullNameWithoutName = new FullName(commandLineWithoutName);
assertFalse(fullNameWithoutName.hasName());

String[] argsWithName = {
"catalog", "details", "--metalake", "metalake", "--name", "Hive_catalog"
};
CommandLine commandLineWithName = new DefaultParser().parse(options, argsWithName);
FullName fullNameWithName = new FullName(commandLineWithName);
assertTrue(fullNameWithName.hasName());
}

@Test
public void hasPartNameMetalake() throws Exception {
String[] args = {"metalake", "details", "--metalake", "metalake"};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.eq;
Expand Down Expand Up @@ -141,6 +142,30 @@ void testCreateTagCommand() {
verify(mockCreate).handle();
}

@Test
void testCreateCommandWithoutTagOption() {
Main.useExit = false;
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(false);

GravitinoCommandLine commandLine =
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.CREATE));

assertThrows(RuntimeException.class, commandLine::handleCommandLine);
verify(commandLine, never())
.newCreateTags(
eq(GravitinoCommandLine.DEFAULT_URL),
eq(false),
eq("metalake_demo"),
isNull(),
isNull());
String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(output, ErrorMessages.MISSING_TAG);
}

@Test
void testCreateTagsCommand() {
CreateTag mockCreate = mock(CreateTag.class);
Expand Down Expand Up @@ -272,6 +297,62 @@ void testSetTagPropertyCommand() {
verify(mockSetProperty).handle();
}

@Test
void testSetTagPropertyCommandWithoutPropertyOption() {
Main.useExit = false;
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true);
when(mockCommandLine.getOptionValues(GravitinoOptions.TAG)).thenReturn(new String[] {"tagA"});
when(mockCommandLine.hasOption(GravitinoOptions.PROPERTY)).thenReturn(false);
when(mockCommandLine.hasOption(GravitinoOptions.VALUE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.VALUE)).thenReturn("value");
GravitinoCommandLine commandLine =
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.SET));

assertThrows(RuntimeException.class, commandLine::handleCommandLine);
verify(commandLine, never())
.newSetTagProperty(
eq(GravitinoCommandLine.DEFAULT_URL),
eq(false),
eq("metalake_demo"),
eq("tagA"),
isNull(),
eq("value"));
String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(output, "The set command only supports tag properties or attaching tags.");
}

@Test
void testSetTagPropertyCommandWithoutValueOption() {
Main.useExit = false;
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true);
when(mockCommandLine.getOptionValues(GravitinoOptions.TAG)).thenReturn(new String[] {"tagA"});
when(mockCommandLine.hasOption(GravitinoOptions.PROPERTY)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.PROPERTY)).thenReturn("property");
when(mockCommandLine.hasOption(GravitinoOptions.VALUE)).thenReturn(false);
GravitinoCommandLine commandLine =
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.SET));

assertThrows(RuntimeException.class, commandLine::handleCommandLine);
verify(commandLine, never())
.newSetTagProperty(
eq(GravitinoCommandLine.DEFAULT_URL),
eq(false),
eq("metalake_demo"),
eq("tagA"),
eq("property"),
isNull());
String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(output, "The set command only supports tag properties or attaching tags.");
}

@Test
void testSetMultipleTagPropertyCommandError() {
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
Expand Down Expand Up @@ -339,6 +420,7 @@ void testDeleteAllTagCommand() {
when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(false);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.FORCE)).thenReturn(true);
when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.NAME)).thenReturn("catalog.schema.table");
GravitinoCommandLine commandLine =
spy(
Expand Down Expand Up @@ -447,6 +529,32 @@ public boolean matches(String[] argument) {
verify(mockTagEntity).handle();
}

@Test
void testTagEntityCommandWithoutName() {
Main.useExit = false;
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(false);
when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true);
when(mockCommandLine.getOptionValues(GravitinoOptions.TAG)).thenReturn(new String[] {"tagA"});
GravitinoCommandLine commandLine =
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.SET));

assertThrows(RuntimeException.class, commandLine::handleCommandLine);
verify(commandLine, never())
.newTagEntity(
eq(GravitinoCommandLine.DEFAULT_URL),
eq(false),
eq("metalake_demo"),
isNull(),
argThat(
argument -> argument != null && argument.length > 0 && "tagA".equals(argument[0])));
String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(output, ErrorMessages.MISSING_NAME);
}

@Test
void testTagsEntityCommand() {
TagEntity mockTagEntity = mock(TagEntity.class);
Expand Down Expand Up @@ -516,6 +624,37 @@ public boolean matches(String[] argument) {
verify(mockUntagEntity).handle();
}

@Test
void testUntagEntityCommandWithoutName() {
Main.useExit = false;
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(false);
when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true);
when(mockCommandLine.getOptionValues(GravitinoOptions.TAG))
.thenReturn(new String[] {"tagA", "tagB"});
GravitinoCommandLine commandLine =
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.REMOVE));

assertThrows(RuntimeException.class, commandLine::handleCommandLine);
verify(commandLine, never())
.newUntagEntity(
eq(GravitinoCommandLine.DEFAULT_URL),
eq(false),
eq("metalake_demo"),
isNull(),
argThat(
argument ->
argument != null
&& argument.length > 0
&& "tagA".equals(argument[0])
&& "tagB".equals(argument[1])));
String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(output, ErrorMessages.MISSING_NAME);
}

@Test
void testUntagsEntityCommand() {
UntagEntity mockUntagEntity = mock(UntagEntity.class);
Expand Down
Loading