Skip to content

Commit

Permalink
[apache#6103] Clean up error messages in Gravitino CLI (apache#6108)
Browse files Browse the repository at this point in the history
What changes were proposed in this pull request?

Clean up the error messages.

### Why are the changes needed?

Put them all in one place and made more consistent.

Fix: apache#6103

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

No

### How was this patch tested?

Tested locally.
  • Loading branch information
justinmclean authored Jan 6, 2025
1 parent e551330 commit 900fef3
Show file tree
Hide file tree
Showing 22 changed files with 109 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,51 +21,67 @@

/* User friendly error messages. */
public class ErrorMessages {
public static final String UNSUPPORTED_COMMAND = "Unsupported or unknown command.";
public static final String UNKNOWN_ENTITY = "Unknown entity.";
public static final String TOO_MANY_ARGUMENTS = "Too many arguments.";
public static final String UNKNOWN_METALAKE = "Unknown metalake name.";
public static final String UNKNOWN_CATALOG = "Unknown catalog name.";
public static final String UNKNOWN_SCHEMA = "Unknown schema name.";
public static final String UNKNOWN_TABLE = "Unknown table name.";
public static final String UNKNOWN_MODEL = "Unknown model name.";
public static final String CATALOG_EXISTS = "Catalog already exists.";
public static final String COLUMN_EXISTS = "Column already exists.";
public static final String FILESET_EXISTS = "Fileset already exists.";
public static final String GROUP_EXISTS = "Group already exists.";
public static final String METALAKE_EXISTS = "Metalake already exists.";
public static final String MODEL_EXISTS = "Model already exists.";
public static final String ROLE_EXISTS = "Role already exists.";
public static final String SCHEMA_EXISTS = "Schema already exists.";
public static final String TABLE_EXISTS = "Table already exists.";
public static final String TAG_EXISTS = "Tag already exists.";
public static final String TOPIC_EXISTS = "Topic already exists.";
public static final String USER_EXISTS = "User already exists.";

public static final String ENTITY_IN_USE = " in use, please disable it first.";

public static final String INVALID_ENABLE_DISABLE =
"Unable to us --enable and --disable at the same time";
public static final String INVALID_OWNER_COMMAND =
"Unsupported combination of options either use --user or --group.";
public static final String INVALID_REMOVE_COMMAND =
"Unsupported combination of options either use --name or --property.";
public static final String INVALID_SET_COMMAND =
"Unsupported combination of options either use --name, --user, --group or --property and --value.";

public static final String HELP_FAILED = "Failed to load help message: ";

public static final String MALFORMED_NAME = "Malformed entity name.";
public static final String MISSING_NAME = "Missing --name option.";
public static final String MISSING_METALAKE = "Missing --metalake option.";
public static final String MISSING_ENTITIES = "Missing required entity names: ";

public static final String MISSING_GROUP = "Missing --group option.";
public static final String MISSING_USER = "Missing --user option.";
public static final String MISSING_METALAKE = "Missing --metalake option.";
public static final String MISSING_NAME = "Missing --name option.";
public static final String MISSING_PROPERTY = "Missing --property option.";
public static final String MISSING_ROLE = "Missing --role option.";
public static final String MISSING_TAG = "Missing --tag option.";
public static final String MISSING_URI = "Missing --uri option.";
public static final String MISSING_PROPERTY = "Missing --property option.";
public static final String MISSING_USER = "Missing --user option.";
public static final String MISSING_VALUE = "Missing --value option.";
public static final String METALAKE_EXISTS = "Metalake already exists.";
public static final String CATALOG_EXISTS = "Catalog already exists.";
public static final String SCHEMA_EXISTS = "Schema already exists.";
public static final String UNKNOWN_USER = "Unknown user.";
public static final String USER_EXISTS = "User already exists.";
public static final String UNKNOWN_GROUP = "Unknown group.";
public static final String GROUP_EXISTS = "Group already exists.";
public static final String UNKNOWN_TAG = "Unknown tag.";

public static final String MULTIPLE_TAG_COMMAND_ERROR =
"Error: The current command only supports one --tag option.";
public static final String TAG_EXISTS = "Tag already exists.";
public static final String UNKNOWN_COLUMN = "Unknown column.";
public static final String COLUMN_EXISTS = "Column already exists.";
public static final String UNKNOWN_TOPIC = "Unknown topic.";
public static final String TOPIC_EXISTS = "Topic already exists.";
public static final String MODEL_EXISTS = "Model already exists.";
public static final String UNKNOWN_FILESET = "Unknown fileset.";
public static final String FILESET_EXISTS = "Fileset already exists.";
public static final String TAG_EMPTY = "Error: Must configure --tag option.";
"This command only supports one --tag option.";

public static final String REGISTER_FAILED = "Failed to register model: ";

public static final String UNKNOWN_CATALOG = "Unknown catalog name.";
public static final String UNKNOWN_COLUMN = "Unknown column name.";
public static final String UNKNOWN_ENTITY = "Unknown entity.";
public static final String UNKNOWN_FILESET = "Unknown fileset name.";
public static final String UNKNOWN_GROUP = "Unknown group.";
public static final String UNKNOWN_METALAKE = "Unknown metalake name.";
public static final String UNKNOWN_MODEL = "Unknown model name.";
public static final String UNKNOWN_PRIVILEGE = "Unknown privilege";
public static final String UNKNOWN_ROLE = "Unknown role.";
public static final String ROLE_EXISTS = "Role already exists.";
public static final String TABLE_EXISTS = "Table already exists.";
public static final String INVALID_SET_COMMAND =
"Unsupported combination of options either use --name, --user, --group or --property and --value.";
public static final String INVALID_REMOVE_COMMAND =
"Unsupported combination of options either use --name or --property.";
public static final String INVALID_OWNER_COMMAND =
"Unsupported combination of options either use --user or --group.";
public static final String UNKNOWN_SCHEMA = "Unknown schema name.";
public static final String UNKNOWN_TABLE = "Unknown table name.";
public static final String UNKNOWN_TAG = "Unknown tag.";
public static final String UNKNOWN_TOPIC = "Unknown topic name.";
public static final String UNKNOWN_USER = "Unknown user.";

public static final String PARSE_ERROR = "Error parsing command line: ";
public static final String TOO_MANY_ARGUMENTS = "Too many arguments.";
public static final String UNSUPPORTED_ACTION = "Entity doesn't support this action.";
public static final String UNSUPPORTED_COMMAND = "Unsupported or unknown command.";
}
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ private void handleMetalakeCommand() {

case CommandActions.UPDATE:
if (line.hasOption(GravitinoOptions.ENABLE) && line.hasOption(GravitinoOptions.DISABLE)) {
System.err.println("Unable to enable and disable at the same time");
System.err.println(ErrorMessages.INVALID_ENABLE_DISABLE);
Main.exit(-1);
}
if (line.hasOption(GravitinoOptions.ENABLE)) {
Expand Down Expand Up @@ -304,7 +304,7 @@ private void handleCatalogCommand() {

case CommandActions.UPDATE:
if (line.hasOption(GravitinoOptions.ENABLE) && line.hasOption(GravitinoOptions.DISABLE)) {
System.err.println("Unable to enable and disable at the same time");
System.err.println(ErrorMessages.INVALID_ENABLE_DISABLE);
Main.exit(-1);
}
if (line.hasOption(GravitinoOptions.ENABLE)) {
Expand Down Expand Up @@ -673,7 +673,7 @@ protected void handleTagCommand() {
}
newTagEntity(url, ignore, metalake, name, tags).handle();
} else {
System.err.println("The set command only supports tag properties or attaching tags.");
System.err.println(ErrorMessages.INVALID_SET_COMMAND);
Main.exit(-1);
}
break;
Expand Down Expand Up @@ -932,7 +932,7 @@ private void handleHelpCommand() {
}
System.out.print(helpMessage.toString());
} catch (IOException e) {
System.err.println("Failed to load help message: " + e.getMessage());
System.err.println(ErrorMessages.HELP_FAILED + e.getMessage());
Main.exit(-1);
}
}
Expand Down Expand Up @@ -1309,7 +1309,7 @@ public String getAuth() {

private void checkEntities(List<String> entities) {
if (!entities.isEmpty()) {
System.err.println("Missing required argument(s): " + COMMA_JOINER.join(entities));
System.err.println(ErrorMessages.MISSING_ENTITIES + COMMA_JOINER.join(entities));
Main.exit(-1);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public static void main(String[] args) {
commandLine.handleSimpleLine();
}
} catch (ParseException exp) {
System.err.println("Error parsing command line: " + exp.getMessage());
System.err.println(ErrorMessages.PARSE_ERROR + exp.getMessage());
GravitinoCommandLine.displayHelp(options);
exit(-1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public static Privilege.Name toName(String privilege) {
case MANAGE_GRANTS:
return Privilege.Name.MANAGE_GRANTS;
default:
System.err.println("Unknown privilege");
System.err.println(ErrorMessages.UNKNOWN_PRIVILEGE + " " + privilege);
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import com.google.common.base.Joiner;
import java.io.File;
import org.apache.gravitino.cli.ErrorMessages;
import org.apache.gravitino.cli.GravitinoConfig;
import org.apache.gravitino.cli.KerberosData;
import org.apache.gravitino.cli.Main;
Expand Down Expand Up @@ -205,6 +206,6 @@ protected <T> void output(T entity) {
}

protected String getMissingEntitiesInfo(String... entities) {
return "Missing required argument(s): " + COMMA_JOINER.join(entities);
return ErrorMessages.MISSING_ENTITIES + COMMA_JOINER.join(entities);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public CreateTag(
@Override
public void handle() {
if (tags == null || tags.length == 0) {
System.err.println(ErrorMessages.TAG_EMPTY);
System.err.println(ErrorMessages.MISSING_TAG);
} else {
boolean hasOnlyOneTag = tags.length == 1;
if (hasOnlyOneTag) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void handle() {
} catch (NoSuchCatalogException err) {
exitWithError(ErrorMessages.UNKNOWN_CATALOG);
} catch (CatalogInUseException catalogInUseException) {
System.err.println(catalog + " in use, please disable it first.");
System.err.println(catalog + ErrorMessages.ENTITY_IN_USE);
} catch (Exception exp) {
exitWithError(exp.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void handle() {
} catch (NoSuchMetalakeException err) {
exitWithError(ErrorMessages.UNKNOWN_METALAKE);
} catch (MetalakeInUseException inUseException) {
System.err.println(metalake + " in use, please disable it first.");
System.err.println(metalake + ErrorMessages.ENTITY_IN_USE);
} catch (Exception exp) {
exitWithError(exp.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void handle() {
}

if (tags == null || tags.length == 0) {
System.err.println(ErrorMessages.TAG_EMPTY);
System.err.println(ErrorMessages.MISSING_TAG);
} else {
boolean hasOnlyOneTag = tags.length == 1;
if (hasOnlyOneTag) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void handle() {

for (String privilege : privileges) {
if (!Privileges.isValid(privilege)) {
System.err.println("Unknown privilege " + privilege);
System.err.println(ErrorMessages.UNKNOWN_PRIVILEGE + " " + privilege);
return;
}
PrivilegeDTO privilegeDTO =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public void handle() {
if (registeredModel != null) {
System.out.println("Successful register " + registeredModel.name() + ".");
} else {
System.err.println("Failed to register model: " + model + ".");
System.err.println(ErrorMessages.REGISTER_FAILED + model + ".");
Main.exit(-1);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void handle() {

for (String privilege : privileges) {
if (!Privileges.isValid(privilege)) {
System.err.println("Unknown privilege " + privilege);
System.err.println(ErrorMessages.UNKNOWN_PRIVILEGE + " " + privilege);
return;
}
PrivilegeDTO privilegeDTO =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,9 @@ void testCatalogDetailsCommandWithoutCatalog() {
String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(
output,
"Missing --name option."
ErrorMessages.MISSING_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ CommandEntities.CATALOG);
}

Expand Down Expand Up @@ -436,6 +436,6 @@ void testCatalogWithDisableAndEnableOptions() {
GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "catalog", false);
verify(commandLine, never())
.newCatalogDisable(GravitinoCommandLine.DEFAULT_URL, false, "melake_demo", "catalog");
assertTrue(errContent.toString().contains("Unable to enable and disable at the same time"));
assertTrue(errContent.toString().contains(ErrorMessages.INVALID_ENABLE_DISABLE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ void testDeleteColumnCommandWithoutCatalog() {
output,
ErrorMessages.MISSING_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ")
.join(
Arrays.asList(
Expand Down Expand Up @@ -496,7 +496,7 @@ void testDeleteColumnCommandWithoutSchema() {
output,
ErrorMessages.MALFORMED_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ")
.join(
Arrays.asList(
Expand Down Expand Up @@ -531,7 +531,7 @@ void testDeleteColumnCommandWithoutTable() {
output,
ErrorMessages.MALFORMED_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ").join(Arrays.asList(CommandEntities.TABLE, CommandEntities.COLUMN)));
}

Expand Down Expand Up @@ -563,7 +563,7 @@ void testDeleteColumnCommandWithoutColumn() {
output,
ErrorMessages.MALFORMED_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ").join(Arrays.asList(CommandEntities.COLUMN)));
}

Expand All @@ -588,7 +588,7 @@ void testListColumnCommandWithoutCatalog() {
output,
ErrorMessages.MISSING_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ")
.join(
Arrays.asList(
Expand Down Expand Up @@ -617,7 +617,7 @@ void testListColumnCommandWithoutSchema() {
output,
ErrorMessages.MALFORMED_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ").join(Arrays.asList(CommandEntities.SCHEMA, CommandEntities.TABLE)));
}

Expand All @@ -643,7 +643,7 @@ void testListColumnCommandWithoutTable() {
output,
ErrorMessages.MALFORMED_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ CommandEntities.TABLE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ void testListFilesetCommandWithoutCatalog() {
output,
ErrorMessages.MISSING_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ").join(Arrays.asList(CommandEntities.CATALOG, CommandEntities.SCHEMA)));
}

Expand All @@ -394,7 +394,7 @@ void testListFilesetCommandWithoutSchema() {
output,
ErrorMessages.MALFORMED_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ").join(Arrays.asList(CommandEntities.SCHEMA)));
}

Expand All @@ -419,7 +419,7 @@ void testFilesetDetailCommandWithoutCatalog() {
output,
ErrorMessages.MISSING_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ")
.join(
Arrays.asList(
Expand Down Expand Up @@ -448,7 +448,7 @@ void testFilesetDetailCommandWithoutSchema() {
output,
ErrorMessages.MALFORMED_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ").join(Arrays.asList(CommandEntities.SCHEMA, CommandEntities.FILESET)));
}

Expand All @@ -474,7 +474,7 @@ void testFilesetDetailCommandWithoutFileset() {
output,
ErrorMessages.MALFORMED_NAME
+ "\n"
+ "Missing required argument(s): "
+ ErrorMessages.MISSING_ENTITIES
+ Joiner.on(", ").join(Arrays.asList(CommandEntities.FILESET)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public void CreateTagWithNoTag() {

Main.main(args);

assertTrue(errContent.toString().contains(ErrorMessages.TAG_EMPTY)); // Expect error
assertTrue(errContent.toString().contains(ErrorMessages.MISSING_TAG)); // Expect error
}

@SuppressWarnings("DefaultCharset")
Expand All @@ -198,6 +198,6 @@ public void DeleteTagWithNoTag() {

Main.main(args);

assertTrue(errContent.toString().contains(ErrorMessages.TAG_EMPTY)); // Expect error
assertTrue(errContent.toString().contains(ErrorMessages.MISSING_TAG)); // Expect error
}
}
Loading

0 comments on commit 900fef3

Please sign in to comment.