From ec73d1c6618177a573c190a06fdff85f3ea43f9c Mon Sep 17 00:00:00 2001 From: pancx Date: Sun, 19 Jan 2025 13:21:07 +0800 Subject: [PATCH] [#6326] improve(CLI): Make CLI more extendable and maintainable. fix some error. --- .../org/apache/gravitino/cli/CatalogCommandHandler.java | 8 ++------ .../org/apache/gravitino/cli/MetalakeCommandHandler.java | 8 ++------ .../org/apache/gravitino/cli/outputs/OutputProperty.java | 6 +++++- .../org/apache/gravitino/cli/outputs/TableFormat.java | 2 +- .../cli/integration/test/TableFormatOutputIT.java | 2 +- .../org/apache/gravitino/cli/output/TestTableFormat.java | 2 +- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/CatalogCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/CatalogCommandHandler.java index 37e6b50461a..a3b0057184d 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/CatalogCommandHandler.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/CatalogCommandHandler.java @@ -39,7 +39,6 @@ public class CatalogCommandHandler extends CommandHandler { private final FullName name; private final String metalake; private String catalog; - private final String outputFormat; /** * Constructs a {@link CatalogCommandHandler} instance. @@ -59,7 +58,6 @@ public CatalogCommandHandler( this.url = getUrl(line); this.name = new FullName(line); this.metalake = name.getMetalakeName(); - this.outputFormat = line.getOptionValue(GravitinoOptions.OUTPUT); } /** Handles the command execution logic based on the provided command. */ @@ -130,8 +128,7 @@ private void handleDetailsCommand() { gravitinoCommandLine.newCatalogAudit(url, ignore, metalake, catalog).validate().handle(); } else { // TODO: move this to GravitinoCommandLine class - OutputProperty property = OutputProperty.defaultOutputProperty(); - property.setOutputFormat(outputFormat); + OutputProperty property = OutputProperty.fromLine(line); gravitinoCommandLine .newCatalogDetails(url, ignore, property, metalake, catalog) .validate() @@ -224,8 +221,7 @@ private void handleUpdateCommand() { /** Handles the "LIST" command. */ private void handleListCommand() { // TODO: move this to GravitinoCommandLine class - OutputProperty property = OutputProperty.defaultOutputProperty(); - property.setOutputFormat(outputFormat); + OutputProperty property = OutputProperty.fromLine(line); gravitinoCommandLine.newListCatalogs(url, ignore, property, metalake).validate().handle(); } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/MetalakeCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/MetalakeCommandHandler.java index 25c02919364..d490822adb9 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/MetalakeCommandHandler.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/MetalakeCommandHandler.java @@ -113,22 +113,18 @@ private boolean executeCommand() { /** Handles the "LIST" command. */ private void handleListCommand() { - String outputFormat = line.getOptionValue(GravitinoOptions.OUTPUT); // TODO: move this to GravitinoCommandLine class - OutputProperty property = OutputProperty.defaultOutputProperty(); - property.setOutputFormat(outputFormat); + OutputProperty property = OutputProperty.fromLine(line); gravitinoCommandLine.newListMetalakes(url, ignore, property).validate().handle(); } /** Handles the "DETAILS" command. */ private void handleDetailsCommand() { + OutputProperty property = OutputProperty.fromLine(line); if (line.hasOption(GravitinoOptions.AUDIT)) { gravitinoCommandLine.newMetalakeAudit(url, ignore, metalake).validate().handle(); } else { // TODO: move this to GravitinoCommandLine class - String outputFormat = line.getOptionValue(GravitinoOptions.OUTPUT); - OutputProperty property = OutputProperty.defaultOutputProperty(); - property.setOutputFormat(outputFormat); gravitinoCommandLine.newMetalakeDetails(url, ignore, property, metalake).validate().handle(); } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/OutputProperty.java b/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/OutputProperty.java index e7e1782691b..f470abad147 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/OutputProperty.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/OutputProperty.java @@ -110,10 +110,14 @@ public static OutputProperty defaultOutputProperty() { */ public static OutputProperty fromLine(CommandLine line) { OutputProperty outputProperty = defaultOutputProperty(); + String outputFormat = line.getOptionValue(GravitinoOptions.OUTPUT); + if (outputFormat != null) { + outputProperty.setOutputFormat(outputFormat); + } + if (line.hasOption(GravitinoOptions.QUIET)) { outputProperty.setQuiet(true); } - // TODO: implement other options. return outputProperty; } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormat.java b/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormat.java index 25ee3ad053a..5cdc5e3ae97 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormat.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormat.java @@ -667,7 +667,7 @@ public String getOutput(Catalog[] catalogs) { output("No metalakes exist.", System.err); return null; } else { - Column columnA = new Column("METALAKE", null, property); + Column columnA = new Column("CATALOG", null, property); Arrays.stream(catalogs).forEach(metalake -> columnA.addCell(metalake.name())); return getTableFormat(columnA); diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/integration/test/TableFormatOutputIT.java b/clients/cli/src/test/java/org/apache/gravitino/cli/integration/test/TableFormatOutputIT.java index 389971cbd76..54c5020fef4 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/integration/test/TableFormatOutputIT.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/integration/test/TableFormatOutputIT.java @@ -171,7 +171,7 @@ public void testCatalogListCommand() { String output = new String(outputStream.toByteArray(), StandardCharsets.UTF_8).trim(); assertEquals( "+---+-----------+\n" - + "| | METALAKE |\n" + + "| | CATALOG |\n" + "+---+-----------+\n" + "| 1 | postgres |\n" + "| 2 | postgres2 |\n" diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/output/TestTableFormat.java b/clients/cli/src/test/java/org/apache/gravitino/cli/output/TestTableFormat.java index 19a608e8ca6..00bff7ad7f5 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/output/TestTableFormat.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/output/TestTableFormat.java @@ -515,7 +515,7 @@ void testCatalogsTableOutput() { String output = new String(outContent.toByteArray(), StandardCharsets.UTF_8).trim(); Assertions.assertEquals( "+---------------+\n" - + "| METALAKE |\n" + + "| CATALOG |\n" + "+---------------+\n" + "| demo_catalog1 |\n" + "| demo_catalog2 |\n"