[ #4943] Update from feedback on Gravitino CLI demo (#5497)

### What changes were proposed in this pull request? Feedback arising from the demo including: - asking users are you sure to any dangerous operations - adding a --force option - remove --metalake from examples - update documentation ### Why are the changes needed? Update from demo. Fix: #5537 ### Does this PR introduce _any_ user-facing change? Delete commands and rename metalake now ask the user are you sure. ### How was this patch tested? Locally.
apache · Nov 12, 2024 · 11eceb9 · 11eceb9
1 parent c3087cd
commit 11eceb9
Show file tree

Hide file tree

Showing 12 changed files with 222 additions and 79 deletions.
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/AreYouSure.java b/clients/cli/src/main/java/org/apache/gravitino/cli/AreYouSure.java
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.cli;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Scanner;
+
+/* Ask are you sure you want to do this? */
+public class AreYouSure {
+
+  /**
+   * Prompts the user with a confirmation message to confirm an action.
+   *
+   * @param force if {@code true}, skips user confirmation and proceeds.
+   * @return {@code true} if the action is to continue {@code false} otherwise.
+   */
+  public static boolean really(boolean force) {
+    Scanner scanner = new Scanner(System.in, StandardCharsets.UTF_8.name());
+
+    /* force option for scripting */
+    if (force) {
+      return true;
+    }
+
+    System.out.println(
+        "This command could result in data loss or other issues. Are you sure you want to do this? (Y/N)");
+    String answer = scanner.next();
+    return answer.equals("Y");
+  }
+}
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
@@ -100,7 +100,7 @@ public class GravitinoCommandLine {
    *
    * @param line Parsed command line object.
    * @param options Available options for the CLI.
-   * @param entity The entity to apply the command to e.g. metlake, catalog, schema, table etc etc.
+   * @param entity The entity to apply the command to e.g. metalake, catalog, schema, table etc etc.
    * @param command The type of command to run i.e. list, details, update, delete, or create.
    */
   public GravitinoCommandLine(CommandLine line, Options options, String entity, String command) {
@@ -204,7 +204,8 @@ private void handleMetalakeCommand() {
       String comment = line.getOptionValue(GravitinoOptions.COMMENT);
       new CreateMetalake(url, ignore, metalake, comment).handle();
     } else if (CommandActions.DELETE.equals(command)) {
-      new DeleteMetalake(url, ignore, metalake).handle();
+      boolean force = line.hasOption(GravitinoOptions.FORCE);
+      new DeleteMetalake(url, ignore, force, metalake).handle();
     } else if (CommandActions.SET.equals(command)) {
       String property = line.getOptionValue(GravitinoOptions.PROPERTY);
       String value = line.getOptionValue(GravitinoOptions.VALUE);
@@ -221,7 +222,8 @@ private void handleMetalakeCommand() {
       }
       if (line.hasOption(GravitinoOptions.RENAME)) {
         String newName = line.getOptionValue(GravitinoOptions.RENAME);
-        new UpdateMetalakeName(url, ignore, metalake, newName).handle();
+        boolean force = line.hasOption(GravitinoOptions.FORCE);
+        new UpdateMetalakeName(url, ignore, force, metalake, newName).handle();
       }
     }
   }
@@ -254,7 +256,8 @@ private void handleCatalogCommand() {
       Map<String, String> propertyMap = new Properties().parse(properties);
       new CreateCatalog(url, ignore, metalake, catalog, provider, comment, propertyMap).handle();
     } else if (CommandActions.DELETE.equals(command)) {
-      new DeleteCatalog(url, ignore, metalake, catalog).handle();
+      boolean force = line.hasOption(GravitinoOptions.FORCE);
+      new DeleteCatalog(url, ignore, force, metalake, catalog).handle();
     } else if (CommandActions.SET.equals(command)) {
       String property = line.getOptionValue(GravitinoOptions.PROPERTY);
       String value = line.getOptionValue(GravitinoOptions.VALUE);
@@ -302,7 +305,8 @@ private void handleSchemaCommand() {
       String comment = line.getOptionValue(GravitinoOptions.COMMENT);
       new CreateSchema(url, ignore, metalake, catalog, schema, comment).handle();
     } else if (CommandActions.DELETE.equals(command)) {
-      new DeleteSchema(url, ignore, metalake, catalog, schema).handle();
+      boolean force = line.hasOption(GravitinoOptions.FORCE);
+      new DeleteSchema(url, ignore, force, metalake, catalog, schema).handle();
     } else if (CommandActions.SET.equals(command)) {
       String property = line.getOptionValue(GravitinoOptions.PROPERTY);
       String value = line.getOptionValue(GravitinoOptions.VALUE);
@@ -341,7 +345,8 @@ private void handleTableCommand() {
     } else if (CommandActions.CREATE.equals(command)) {
       // TODO
     } else if (CommandActions.DELETE.equals(command)) {
-      new DeleteTable(url, ignore, metalake, catalog, schema, table).handle();
+      boolean force = line.hasOption(GravitinoOptions.FORCE);
+      new DeleteTable(url, ignore, force, metalake, catalog, schema, table).handle();
     }
   }
 
@@ -359,7 +364,8 @@ protected void handleUserCommand() {
     } else if (CommandActions.CREATE.equals(command)) {
       new CreateUser(url, ignore, metalake, user).handle();
     } else if (CommandActions.DELETE.equals(command)) {
-      new DeleteUser(url, ignore, metalake, user).handle();
+      boolean force = line.hasOption(GravitinoOptions.FORCE);
+      new DeleteUser(url, ignore, force, metalake, user).handle();
     }
   }
 
@@ -377,7 +383,8 @@ protected void handleGroupCommand() {
     } else if (CommandActions.CREATE.equals(command)) {
       new CreateGroup(url, ignore, metalake, group).handle();
     } else if (CommandActions.DELETE.equals(command)) {
-      new DeleteGroup(url, ignore, metalake, group).handle();
+      boolean force = line.hasOption(GravitinoOptions.FORCE);
+      new DeleteGroup(url, ignore, force, metalake, group).handle();
     }
   }
 
@@ -400,7 +407,8 @@ protected void handleTagCommand() {
       String comment = line.getOptionValue(GravitinoOptions.COMMENT);
       new CreateTag(url, ignore, metalake, tag, comment).handle();
     } else if (CommandActions.DELETE.equals(command)) {
-      new DeleteTag(url, ignore, metalake, tag).handle();
+      boolean force = line.hasOption(GravitinoOptions.FORCE);
+      new DeleteTag(url, ignore, force, metalake, tag).handle();
     } else if (CommandActions.SET.equals(command)) {
       String property = line.getOptionValue(GravitinoOptions.PROPERTY);
       String value = line.getOptionValue(GravitinoOptions.VALUE);

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
@@ -41,6 +41,7 @@ public class GravitinoOptions {
   public static final String GROUP = "group";
   public static final String TAG = "tag";
   public static final String AUDIT = "audit";
+  public static final String FORCE = "force";
 
   /**
    * Builds and returns the CLI options for Gravitino.
@@ -77,6 +78,9 @@ public Options options() {
         Option.builder("p").longOpt(PROPERTIES).desc("property name/value pairs").hasArgs().build();
     options.addOption(properties);
 
+    // Force delete entity and rename metalake operations
+    options.addOption(createSimpleOption("f", FORCE, "force operation"));
+
     return options;
   }
 

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteCatalog.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteCatalog.java
@@ -19,6 +19,7 @@
 
 package org.apache.gravitino.cli.commands;
 
+import org.apache.gravitino.cli.AreYouSure;
 import org.apache.gravitino.cli.ErrorMessages;
 import org.apache.gravitino.client.GravitinoClient;
 import org.apache.gravitino.exceptions.NoSuchCatalogException;
@@ -28,17 +29,21 @@ public class DeleteCatalog extends Command {
 
   protected final String metalake;
   protected final String catalog;
+  protected final boolean force;
 
   /**
    * Delete a catalog.
    *
    * @param url The URL of the Gravitino server.
    * @param ignoreVersions If true don't check the client/server versions match.
+   * @param force Force operation.
    * @param metalake The name of the metalake.
    * @param catalog The name of the catalog.
    */
-  public DeleteCatalog(String url, boolean ignoreVersions, String metalake, String catalog) {
+  public DeleteCatalog(
+      String url, boolean ignoreVersions, boolean force, String metalake, String catalog) {
     super(url, ignoreVersions);
+    this.force = force;
     this.metalake = metalake;
     this.catalog = catalog;
   }
@@ -48,6 +53,10 @@ public DeleteCatalog(String url, boolean ignoreVersions, String metalake, String
   public void handle() {
     boolean deleted = false;
 
+    if (!AreYouSure.really(force)) {
+      return;
+    }
+
     try {
       GravitinoClient client = buildClient(metalake);
       deleted = client.dropCatalog(catalog);

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteGroup.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteGroup.java
@@ -19,6 +19,7 @@
 
 package org.apache.gravitino.cli.commands;
 
+import org.apache.gravitino.cli.AreYouSure;
 import org.apache.gravitino.cli.ErrorMessages;
 import org.apache.gravitino.client.GravitinoClient;
 import org.apache.gravitino.exceptions.NoSuchGroupException;
@@ -28,17 +29,21 @@ public class DeleteGroup extends Command {
 
   protected final String metalake;
   protected final String group;
+  protected final boolean force;
 
   /**
    * Delete a group.
    *
    * @param url The URL of the Gravitino server.
    * @param ignoreVersions If true don't check the client/server versions match.
+   * @param force Force operation.
    * @param metalake The name of the metalake.
    * @param group The name of the group.
    */
-  public DeleteGroup(String url, boolean ignoreVersions, String metalake, String group) {
+  public DeleteGroup(
+      String url, boolean ignoreVersions, boolean force, String metalake, String group) {
     super(url, ignoreVersions);
+    this.force = force;
     this.metalake = metalake;
     this.group = group;
   }
@@ -48,6 +53,10 @@ public DeleteGroup(String url, boolean ignoreVersions, String metalake, String g
   public void handle() {
     boolean deleted = false;
 
+    if (!AreYouSure.really(force)) {
+      return;
+    }
+
     try {
       GravitinoClient client = buildClient(metalake);
       deleted = client.removeGroup(group);

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteMetalake.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteMetalake.java
@@ -19,29 +19,38 @@
 
 package org.apache.gravitino.cli.commands;
 
+import org.apache.gravitino.cli.AreYouSure;
 import org.apache.gravitino.cli.ErrorMessages;
 import org.apache.gravitino.client.GravitinoAdminClient;
 import org.apache.gravitino.exceptions.NoSuchMetalakeException;
 
 public class DeleteMetalake extends Command {
   protected final String metalake;
+  protected final boolean force;
 
   /**
    * Delete a metalake.
    *
    * @param url The URL of the Gravitino server.
    * @param ignoreVersions If true don't check the client/server versions match.
+   * @param force Force operation.
    * @param metalake The name of the metalake.
    */
-  public DeleteMetalake(String url, boolean ignoreVersions, String metalake) {
+  public DeleteMetalake(String url, boolean ignoreVersions, boolean force, String metalake) {
     super(url, ignoreVersions);
+    this.force = force;
     this.metalake = metalake;
   }
 
   /** Delete a metalake. */
   @Override
   public void handle() {
     boolean deleted = false;
+
+    if (!AreYouSure.really(force)) {
+      return;
+    }
+
     try {
       GravitinoAdminClient client = buildAdminClient();
       deleted = client.dropMetalake(metalake);

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteSchema.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteSchema.java
@@ -19,6 +19,7 @@
 
 package org.apache.gravitino.cli.commands;
 
+import org.apache.gravitino.cli.AreYouSure;
 import org.apache.gravitino.cli.ErrorMessages;
 import org.apache.gravitino.client.GravitinoClient;
 import org.apache.gravitino.exceptions.NoSuchCatalogException;
@@ -30,19 +31,27 @@ public class DeleteSchema extends Command {
   protected final String metalake;
   protected final String catalog;
   protected final String schema;
+  protected final boolean force;
 
   /**
    * Delete a schema.
    *
    * @param url The URL of the Gravitino server.
    * @param ignoreVersions If true don't check the client/server versions match.
+   * @param force Force operation.
    * @param metalake The name of the metalake.
    * @param catalog The name of the catalog.
    * @param schema The name of the schema.
    */
   public DeleteSchema(
-      String url, boolean ignoreVersions, String metalake, String catalog, String schema) {
+      String url,
+      boolean ignoreVersions,
+      boolean force,
+      String metalake,
+      String catalog,
+      String schema) {
     super(url, ignoreVersions);
+    this.force = force;
     this.metalake = metalake;
     this.catalog = catalog;
     this.schema = schema;
@@ -53,6 +62,10 @@ public DeleteSchema(
   public void handle() {
     boolean deleted = false;
 
+    if (!AreYouSure.really(force)) {
+      return;
+    }
+
     try {
       GravitinoClient client = buildClient(metalake);
       deleted = client.loadCatalog(catalog).asSchemas().dropSchema(schema, false);

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteTable.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteTable.java
@@ -20,6 +20,7 @@
 package org.apache.gravitino.cli.commands;
 
 import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.cli.AreYouSure;
 import org.apache.gravitino.cli.ErrorMessages;
 import org.apache.gravitino.client.GravitinoClient;
 import org.apache.gravitino.exceptions.NoSuchCatalogException;
@@ -33,12 +34,14 @@ public class DeleteTable extends Command {
   protected final String catalog;
   protected final String schema;
   protected final String table;
+  protected final boolean force;
 
   /**
    * Delete a table.
    *
    * @param url The URL of the Gravitino server.
    * @param ignoreVersions If true don't check the client/server versions match.
+   * @param force Force operation.
    * @param metalake The name of the metalake.
    * @param catalog The name of the catalog.
    * @param schema The name of the schema.
@@ -47,11 +50,13 @@ public class DeleteTable extends Command {
   public DeleteTable(
       String url,
       boolean ignoreVersions,
+      boolean force,
       String metalake,
       String catalog,
       String schema,
       String table) {
     super(url, ignoreVersions);
+    this.force = force;
     this.metalake = metalake;
     this.catalog = catalog;
     this.schema = schema;
@@ -63,6 +68,10 @@ public DeleteTable(
   public void handle() {
     boolean deleted = false;
 
+    if (!AreYouSure.really(force)) {
+      return;
+    }
+
     try {
       GravitinoClient client = buildClient(metalake);
       NameIdentifier name = NameIdentifier.of(schema, table);