[incubator-uniffle] branch master updated: [#768] [Follow Up] feat(cli): Cli method for blacklist update. (#968)

[xi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

xianjin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new 6dbb2899 [#768] [Follow Up] feat(cli): Cli method for blacklist update. (#968)
6dbb2899 is described below

commit 6dbb289903eb8502379a948f843ac90c4c14d7b0
Author: slfan1989 <55...@users.noreply.github.com>
AuthorDate: Sat Jul 1 21:54:40 2023 +0800

    [#768] [Follow Up] feat(cli): Cli method for blacklist update. (#968)
    
    ### What changes were proposed in this pull request?
    
    While reading the code, I found some minor issues, this PR will fix them.
    
    - Issue1: When using the `uniffle admin-cli` command without any options, it should print the help information.
    
    > before modification
    
    <img width="575" alt="image" src="https://github.com/apache/incubator-uniffle/assets/55643692/8324e9fa-8bf7-4b8c-bbd4-76795fa8e105">
    
    > after modification
    <img width="660" alt="image" src="https://github.com/apache/incubator-uniffle/assets/55643692/8e3b53a8-faec-4d38-ae97-a420ba48c3fa">
    
    - Issue2: The comment information is incorrect.
    
    ### Why are the changes needed?
    
    Improve code readability and enhance code functionality.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    Test environment verification.
---
 .../java/org/apache/uniffle/cli/UniffleAdminCLI.java | 20 +++++++++++++-------
 .../org/apache/uniffle/client/RestClientImpl.java    |  1 -
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/cli/src/main/java/org/apache/uniffle/cli/UniffleAdminCLI.java b/cli/src/main/java/org/apache/uniffle/cli/UniffleAdminCLI.java
index 67d41463..f8c15f8f 100644
--- a/cli/src/main/java/org/apache/uniffle/cli/UniffleAdminCLI.java
+++ b/cli/src/main/java/org/apache/uniffle/cli/UniffleAdminCLI.java
@@ -35,7 +35,7 @@ public class UniffleAdminCLI extends AbstractCustomCommandLine {
   private final Options allOptions;
   private final Option refreshCheckerCli;
   private final Option coordinatorHost;
-  private final Option coordPort;
+  private final Option coordinatorPort;
   private final Option ssl;
 
   private final Option help;
@@ -49,13 +49,13 @@ public class UniffleAdminCLI extends AbstractCustomCommandLine {
         false, "Help for the Uniffle Admin CLI.");
     coordinatorHost = new Option(shortPrefix + "s", longPrefix + "coordinatorHost",
         true, "This is coordinator server host.");
-    coordPort = new Option(shortPrefix + "p", longPrefix + "port",
+    coordinatorPort = new Option(shortPrefix + "p", longPrefix + "coordinatorPort",
         true, "This is coordinator server port.");
-    ssl = new Option(null, longPrefix + "ssl", false, "use SSL");
+    ssl = new Option(null, longPrefix + "ssl", false, "use SSL.");
 
     allOptions.addOption(refreshCheckerCli);
     allOptions.addOption(coordinatorHost);
-    allOptions.addOption(coordPort);
+    allOptions.addOption(coordinatorPort);
     allOptions.addOption(ssl);
     allOptions.addOption(help);
   }
@@ -68,14 +68,19 @@ public class UniffleAdminCLI extends AbstractCustomCommandLine {
   public int run(String[] args) throws UniffleCliArgsException {
     final CommandLine cmd = parseCommandLineOptions(args, true);
 
+    if (args != null && args.length < 1) {
+      printUsage();
+      return 1;
+    }
+
     if (cmd.hasOption(help.getOpt())) {
       printUsage();
       return 0;
     }
 
-    if (cmd.hasOption(coordinatorHost.getOpt()) && cmd.hasOption(coordPort.getOpt())) {
+    if (cmd.hasOption(coordinatorHost.getOpt()) && cmd.hasOption(coordinatorPort.getOpt())) {
       String host = cmd.getOptionValue(coordinatorHost.getOpt()).trim();
-      int port = Integer.parseInt(cmd.getOptionValue(coordPort.getOpt()).trim());
+      int port = Integer.parseInt(cmd.getOptionValue(coordinatorPort.getOpt()).trim());
       String hostUrl;
       if (cmd.hasOption(ssl.getOpt())) {
         hostUrl = String.format("https://%s:%d", host, port);
@@ -106,7 +111,8 @@ public class UniffleAdminCLI extends AbstractCustomCommandLine {
   public void addRunOptions(Options baseOptions) {
     baseOptions.addOption(refreshCheckerCli);
     baseOptions.addOption(coordinatorHost);
-    baseOptions.addOption(coordPort);
+    baseOptions.addOption(coordinatorPort);
+    baseOptions.addOption(ssl);
   }
 
   @Override
diff --git a/cli/src/main/java/org/apache/uniffle/client/RestClientImpl.java b/cli/src/main/java/org/apache/uniffle/client/RestClientImpl.java
index 87bf8952..f4acbb49 100644
--- a/cli/src/main/java/org/apache/uniffle/client/RestClientImpl.java
+++ b/cli/src/main/java/org/apache/uniffle/client/RestClientImpl.java
@@ -92,7 +92,6 @@ public class RestClientImpl implements RestClient {
       response = httpclient.execute(httpRequest, responseHandler);
       LOG.debug("Response: {}", response);
     } catch (ConnectException | ConnectTimeoutException | NoHttpResponseException e) {
-      // net exception can be retried by connecting to other Kyuubi server
       throw new UniffleRestException("Api request failed for " + uri.toString(), e);
     } catch (UniffleRestException rethrow) {
       throw rethrow;