Repository: sqoop
Updated Branches:
  refs/heads/SQOOP-1367 e50f69110 -> d8e336b3e


SQOOP-1438: Sqoop2: Take advantage of isRequired flag in OptionParser in Sqoop 
shell


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

Branch: refs/heads/SQOOP-1367
Commit: d8e336b3eae6ce1474f5de4c3816f43deb87258f
Parents: e50f691
Author: Abraham Elmahrek <[email protected]>
Authored: Fri Aug 15 10:52:46 2014 -0700
Committer: Abraham Elmahrek <[email protected]>
Committed: Fri Aug 15 10:52:46 2014 -0700

----------------------------------------------------------------------
 .../sqoop/shell/CloneConnectionFunction.java      | 10 +---------
 .../org/apache/sqoop/shell/CloneJobFunction.java  | 10 +---------
 .../sqoop/shell/CreateConnectionFunction.java     | 10 +---------
 .../org/apache/sqoop/shell/CreateJobFunction.java | 15 ++-------------
 .../sqoop/shell/DeleteConnectionFunction.java     | 10 +---------
 .../org/apache/sqoop/shell/DeleteJobFunction.java | 10 +---------
 .../sqoop/shell/DisableConnectionFunction.java    | 10 +---------
 .../sqoop/shell/EnableConnectionFunction.java     | 10 +---------
 .../org/apache/sqoop/shell/EnableJobFunction.java | 10 +---------
 .../org/apache/sqoop/shell/SetOptionFunction.java | 15 ++-------------
 .../sqoop/shell/UpdateConnectionFunction.java     | 10 +---------
 .../org/apache/sqoop/shell/UpdateJobFunction.java | 10 +---------
 .../sqoop/shell/utils/ThrowableDisplayer.java     | 18 ++++++++++++------
 13 files changed, 26 insertions(+), 122 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java
----------------------------------------------------------------------
diff --git 
a/shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java
index 9cce17b..d912c1c 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java
@@ -45,20 +45,12 @@ public class CloneConnectionFunction extends SqoopFunction {
       .withDescription(resourceString(Constants.RES_PROMPT_CONN_ID))
       .withLongOpt(Constants.OPT_XID)
       .hasArg()
+      .isRequired()
       .create(Constants.OPT_XID_CHAR)
     );
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_XID)) {
-      printlnResource(Constants.RES_ARGS_XID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   @SuppressWarnings("unchecked")
   public Object executeFunction(CommandLine line, boolean isInteractive) 
throws IOException {
     return cloneConnection(getLong(line, Constants.OPT_XID), 
line.getArgList(), isInteractive);

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java
----------------------------------------------------------------------
diff --git a/shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java
index 74c863d..1b8f2b8 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java
@@ -45,19 +45,11 @@ public class CloneJobFunction extends SqoopFunction {
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_JOB_ID))
       .withLongOpt(Constants.OPT_JID)
+      .isRequired()
       .hasArg()
       .create(Constants.OPT_JID_CHAR));
   }
 
-  @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_JID)) {
-      printlnResource(Constants.RES_ARGS_JID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
   @SuppressWarnings("unchecked")
   public Object executeFunction(CommandLine line, boolean isInteractive) 
throws IOException {
     return cloneJob(getLong(line, Constants.OPT_JID), line.getArgList(), 
isInteractive);

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/CreateConnectionFunction.java
----------------------------------------------------------------------
diff --git 
a/shell/src/main/java/org/apache/sqoop/shell/CreateConnectionFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/CreateConnectionFunction.java
index ae26035..92a8aa5 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/CreateConnectionFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/CreateConnectionFunction.java
@@ -44,20 +44,12 @@ public class CreateConnectionFunction extends SqoopFunction 
{
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_CONNECTOR_ID))
       .withLongOpt(Constants.OPT_CID)
+      .isRequired()
       .hasArg()
       .create(Constants.OPT_CID_CHAR));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_CID)) {
-      printlnResource(Constants.RES_ARGS_CID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   @SuppressWarnings("unchecked")
   public Object executeFunction(CommandLine line, boolean isInteractive) 
throws IOException {
     return createConnection(getLong(line, Constants.OPT_CID), 
line.getArgList(), isInteractive);

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java
----------------------------------------------------------------------
diff --git a/shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java
index 4ddcc12..9c558e2 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java
@@ -45,31 +45,20 @@ public class CreateJobFunction extends  SqoopFunction {
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_CONN_ID))
       .withLongOpt(Constants.OPT_FROM)
+      .isRequired()
       .hasArg()
       .create(Constants.OPT_FXID_CHAR)
     );
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_CONN_ID))
       .withLongOpt(Constants.OPT_TO)
+      .isRequired()
       .hasArg()
       .create(Constants.OPT_TXID_CHAR)
     );
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_FROM)) {
-      printlnResource(Constants.RES_ARGS_FROM_MISSING);
-      return false;
-    }
-    if (!line.hasOption(Constants.OPT_TO)) {
-      printlnResource(Constants.RES_ARGS_TO_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   @SuppressWarnings("unchecked")
   public Object executeFunction(CommandLine line, boolean isInteractive) 
throws IOException {
     return createJob(getLong(line, Constants.OPT_FROM),

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java
----------------------------------------------------------------------
diff --git 
a/shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java
index 12f0083..1eb7e51 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java
@@ -34,20 +34,12 @@ public class DeleteConnectionFunction extends SqoopFunction 
{
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_CONN_ID))
       .withLongOpt(Constants.OPT_XID)
+      .isRequired()
       .hasArg()
       .create('x'));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_FROM)) {
-      printlnResource(Constants.RES_ARGS_XID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   public Object executeFunction(CommandLine line, boolean isInteractive) {
     client.deleteConnection(getLong(line, Constants.OPT_XID));
     return Status.FINE;

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/DeleteJobFunction.java
----------------------------------------------------------------------
diff --git a/shell/src/main/java/org/apache/sqoop/shell/DeleteJobFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/DeleteJobFunction.java
index f3ae59d..5d48c91 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/DeleteJobFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/DeleteJobFunction.java
@@ -35,20 +35,12 @@ public class DeleteJobFunction extends SqoopFunction {
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_JOB_ID))
       .withLongOpt(Constants.OPT_JID)
+      .isRequired()
       .hasArg()
       .create('j'));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_JID)) {
-      printlnResource(Constants.RES_ARGS_JID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   public Object executeFunction(CommandLine line, boolean isInteractive) {
     client.deleteJob(getLong(line, Constants.OPT_JID));
     return Status.FINE;

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java
----------------------------------------------------------------------
diff --git 
a/shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java
index 6f07c32..816ff75 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java
@@ -34,20 +34,12 @@ public class DisableConnectionFunction extends 
SqoopFunction {
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_CONN_ID))
       .withLongOpt(Constants.OPT_XID)
+      .isRequired()
       .hasArg()
       .create('x'));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_XID)) {
-      printlnResource(Constants.RES_ARGS_XID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   public Object executeFunction(CommandLine line, boolean isInteractive) {
     client.enableConnection(getLong(line, Constants.OPT_XID), false);
     return Status.FINE;

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java
----------------------------------------------------------------------
diff --git 
a/shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java
index 9256712..174c3df 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java
@@ -34,20 +34,12 @@ public class EnableConnectionFunction extends SqoopFunction 
{
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_CONN_ID))
       .withLongOpt(Constants.OPT_XID)
+      .isRequired()
       .hasArg()
       .create('x'));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_XID)) {
-      printlnResource(Constants.RES_ARGS_XID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   public Object executeFunction(CommandLine line, boolean isInteractive) {
     client.enableConnection(getLong(line, Constants.OPT_XID), true);
     return Status.FINE;

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/EnableJobFunction.java
----------------------------------------------------------------------
diff --git a/shell/src/main/java/org/apache/sqoop/shell/EnableJobFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/EnableJobFunction.java
index e4db06e..8575a84 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/EnableJobFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/EnableJobFunction.java
@@ -35,20 +35,12 @@ public class EnableJobFunction extends SqoopFunction {
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_JOB_ID))
       .withLongOpt(Constants.OPT_JID)
+      .isRequired()
       .hasArg()
       .create('j'));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_JID)) {
-      printlnResource(Constants.RES_ARGS_JID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   public Object executeFunction(CommandLine line, boolean isInteractive) {
     client.enableJob(getLong(line, Constants.OPT_JID), true);
     return Status.FINE;

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/SetOptionFunction.java
----------------------------------------------------------------------
diff --git a/shell/src/main/java/org/apache/sqoop/shell/SetOptionFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/SetOptionFunction.java
index 700f646..ccc067f 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/SetOptionFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/SetOptionFunction.java
@@ -34,27 +34,16 @@ public class SetOptionFunction extends SqoopFunction {
     this.addOption(OptionBuilder.hasArg()
       .withDescription(resourceString(Constants.RES_SET_PROMPT_OPT_NAME))
       .withLongOpt(Constants.OPT_NAME)
+      .isRequired()
       .create(Constants.OPT_NAME_CHAR));
     this.addOption(OptionBuilder.hasArg()
       .withDescription(resourceString(Constants.RES_SET_PROMPT_OPT_VALUE))
       .withLongOpt(Constants.OPT_VALUE)
+      .isRequired()
       .create(Constants.OPT_VALUE_CHAR));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_NAME)) {
-      printlnResource(Constants.RES_ARGS_NAME_MISSING);
-      return false;
-    }
-    if (!line.hasOption(Constants.OPT_VALUE)) {
-      printlnResource(Constants.RES_ARGS_VALUE_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   public Object executeFunction(CommandLine line, boolean isInteractive) {
     if (!line.hasOption(Constants.OPT_NAME)) {
       printlnResource(Constants.RES_ARGS_NAME_MISSING);

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java
----------------------------------------------------------------------
diff --git 
a/shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java
index 8fa1bda..405b0ea 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java
@@ -44,20 +44,12 @@ public class UpdateConnectionFunction extends SqoopFunction 
{
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_CONN_ID))
       .withLongOpt(Constants.OPT_XID)
+      .isRequired()
       .hasArg()
       .create(Constants.OPT_XID_CHAR));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_XID)) {
-      printlnResource(Constants.RES_ARGS_XID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   @SuppressWarnings("unchecked")
   public Object executeFunction(CommandLine line, boolean isInteractive) 
throws IOException {
     return updateConnection(getLong(line, Constants.OPT_XID), 
line.getArgList(), isInteractive);

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java
----------------------------------------------------------------------
diff --git a/shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java 
b/shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java
index fbaf661..9f5366d 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java
@@ -45,20 +45,12 @@ public class UpdateJobFunction extends SqoopFunction {
     this.addOption(OptionBuilder
       .withDescription(resourceString(Constants.RES_PROMPT_JOB_ID))
       .withLongOpt(Constants.OPT_JID)
+      .isRequired()
       .hasArg()
       .create(Constants.OPT_JID_CHAR));
   }
 
   @Override
-  public boolean validateArgs(CommandLine line) {
-    if (!line.hasOption(Constants.OPT_JID)) {
-      printlnResource(Constants.RES_ARGS_JID_MISSING);
-      return false;
-    }
-    return true;
-  }
-
-  @Override
   @SuppressWarnings("unchecked")
   public Object executeFunction(CommandLine line, boolean isInteractive) 
throws IOException {
     return updateJob(getLong(line, Constants.OPT_JID), line.getArgList(), 
isInteractive);

http://git-wip-us.apache.org/repos/asf/sqoop/blob/d8e336b3/shell/src/main/java/org/apache/sqoop/shell/utils/ThrowableDisplayer.java
----------------------------------------------------------------------
diff --git 
a/shell/src/main/java/org/apache/sqoop/shell/utils/ThrowableDisplayer.java 
b/shell/src/main/java/org/apache/sqoop/shell/utils/ThrowableDisplayer.java
index 6026a95..b9c8cad 100644
--- a/shell/src/main/java/org/apache/sqoop/shell/utils/ThrowableDisplayer.java
+++ b/shell/src/main/java/org/apache/sqoop/shell/utils/ThrowableDisplayer.java
@@ -40,17 +40,23 @@ public class ThrowableDisplayer {
    * @param t Throwable to be displayed
    */
   public static void errorHook(Throwable t) {
-    println("@|red Exception has occurred during processing command |@");
-
-    // If this is server exception from server
-    if(t instanceof SqoopException
-      && ((SqoopException)t).getErrorCode() == ShellError.SHELL_0006) {
-      print("@|red Server has returned exception: |@");
+    // Based on the kind of exception we are dealing with, let's provide 
different user experince
+    if(t instanceof SqoopException && ((SqoopException)t).getErrorCode() == 
ShellError.SHELL_0006) {
+      println("@|red Server has returned exception: |@");
       printThrowable(t.getCause(), isVerbose());
+    } else if(t instanceof SqoopException && 
((SqoopException)t).getErrorCode() == ShellError.SHELL_0003) {
+      print("@|red Invalid command invocation: |@");
+      // In most cases the cause will be actual parsing error, so let's print 
that alone
+      if (t.getCause() != null) {
+        println(t.getCause().getMessage());
+      } else {
+        println(t.getMessage());
+      }
     } else if(t.getClass() == MissingPropertyException.class) {
       print("@|red Unknown command: |@");
       println(t.getMessage());
     } else {
+      println("@|red Exception has occurred during processing command |@");
       printThrowable(t, isVerbose());
     }
   }

Reply via email to