You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by el...@apache.org on 2015/01/14 00:19:19 UTC

[2/6] accumulo git commit: ACCUMULO-3475 Remove configError and use config(String...) retval

ACCUMULO-3475 Remove configError and use config(String...) retval

The return value of config(String...) is ignored, using the
configError member instead. We can get rid of that member and
use the retval instead to denote the same thing.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/7651b777
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/7651b777
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/7651b777

Branch: refs/heads/1.6
Commit: 7651b77724481811b967b36ec6d164119edcd5e1
Parents: cfb832a
Author: Josh Elser <jo...@gmail.com>
Authored: Tue Jan 13 15:18:13 2015 -0500
Committer: Josh Elser <jo...@gmail.com>
Committed: Tue Jan 13 15:47:02 2015 -0500

----------------------------------------------------------------------
 .../accumulo/core/client/mock/MockShell.java    | 13 +++++-----
 .../apache/accumulo/core/util/shell/Shell.java  | 27 +++++++++++---------
 .../shell/command/FormatterCommandTest.java     |  4 ++-
 3 files changed, 25 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/7651b777/core/src/main/java/org/apache/accumulo/core/client/mock/MockShell.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/client/mock/MockShell.java b/core/src/main/java/org/apache/accumulo/core/client/mock/MockShell.java
index 5ff340b..0fbe879 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/mock/MockShell.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/mock/MockShell.java
@@ -44,15 +44,19 @@ public class MockShell extends Shell {
     this.writer = writer;
   }
 
+  @Override
   public boolean config(String... args) {
-    configError = super.config(args);
+    // If configuring the shell failed, fail quickly
+    if (!super.config(args)) {
+      return false;
+    }
 
     // Update the ConsoleReader with the input and output "redirected"
     try {
       this.reader = new ConsoleReader(in, writer);
     } catch (Exception e) {
       printException(e);
-      configError = true;
+      return false;
     }
 
     // Don't need this for testing purposes
@@ -61,7 +65,7 @@ public class MockShell extends Shell {
 
     // Make the parsing from the client easier;
     this.verbose = false;
-    return configError;
+    return true;
   }
 
   @Override
@@ -71,9 +75,6 @@ public class MockShell extends Shell {
   }
 
   public int start() throws IOException {
-    if (configError)
-      return 1;
-
     String input;
     if (isVerbose())
       printInfo();

http://git-wip-us.apache.org/repos/asf/accumulo/blob/7651b777/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java b/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
index cc2053f..808d340 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
@@ -185,7 +185,6 @@ public class Shell extends ShellOptions {
   private Token rootToken;
   public final Map<String,Command> commandFactory = new TreeMap<String,Command>();
   public final Map<String,Command[]> commandGrouping = new TreeMap<String,Command[]>();
-  protected boolean configError = false;
 
   // exit if true
   private boolean exit = false;
@@ -215,7 +214,11 @@ public class Shell extends ShellOptions {
     this.writer = writer;
   }
 
-  // Not for client use
+  /**
+   * Configures the shell using the provided options. Not for client use.
+   *
+   * @return true if the shell was successfully configured, false otherwise.
+   */
   public boolean config(String... args) {
 
     CommandLine cl;
@@ -225,9 +228,9 @@ public class Shell extends ShellOptions {
         throw new ParseException("Unrecognized arguments: " + cl.getArgList());
 
       if (cl.hasOption(helpOpt.getOpt())) {
-        configError = true;
         printHelp("shell", SHELL_DESCRIPTION, opts);
-        return true;
+        exitCode = 0;
+        return false;
       }
 
       setDebugging(cl.hasOption(debugOption.getLongOpt()));
@@ -238,10 +241,10 @@ public class Shell extends ShellOptions {
         throw new MissingArgumentException(zooKeeperInstance);
 
     } catch (Exception e) {
-      configError = true;
       printException(e);
       printHelp("shell", SHELL_DESCRIPTION, opts);
-      return true;
+      exitCode = 1;
+      return false;
     }
 
     // get the options that were parsed
@@ -316,7 +319,8 @@ public class Shell extends ShellOptions {
 
     } catch (Exception e) {
       printException(e);
-      configError = true;
+      exitCode = 1;
+      return false;
     }
 
     // decide whether to execute commands from a file and quit
@@ -373,7 +377,7 @@ public class Shell extends ShellOptions {
     for (Command cmd : otherCommands) {
       commandFactory.put(cmd.getName(), cmd);
     }
-    return configError;
+    return true;
   }
 
   protected void setInstance(CommandLine cl) {
@@ -408,15 +412,14 @@ public class Shell extends ShellOptions {
 
   public static void main(String args[]) throws IOException {
     Shell shell = new Shell();
-    shell.config(args);
+    if (!shell.config(args)) {
+      System.exit(shell.getExitCode());
+    }
 
     System.exit(shell.start());
   }
 
   public int start() throws IOException {
-    if (configError)
-      return 1;
-
     String input;
     if (isVerbose())
       printInfo();

http://git-wip-us.apache.org/repos/asf/accumulo/blob/7651b777/core/src/test/java/org/apache/accumulo/core/util/shell/command/FormatterCommandTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/command/FormatterCommandTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/command/FormatterCommandTest.java
index b99cf60..c615e48 100644
--- a/core/src/test/java/org/apache/accumulo/core/util/shell/command/FormatterCommandTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/util/shell/command/FormatterCommandTest.java
@@ -16,6 +16,8 @@
  */
 package org.apache.accumulo.core.util.shell.command;
 
+import static org.junit.Assert.assertTrue;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.StringWriter;
@@ -57,7 +59,7 @@ public class FormatterCommandTest {
     writer = new StringWriter();
 
     final MockShell shell = new MockShell(in, writer);
-    shell.config(args);
+    assertTrue("Failed to configure shell without error", shell.config(args));
 
     // Can't call createtable in the shell with MockAccumulo
     shell.getConnector().tableOperations().create("test");