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 <[email protected]>
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;
