From 2f94baeb96a38c937c5594766153f1ef6f1df7b8 Mon Sep 17 00:00:00 2001 From: Abraham Elmahrek Date: Fri, 15 Aug 2014 10:52:46 -0700 Subject: [PATCH] SQOOP-1438: Sqoop2: Take advantage of isRequired flag in OptionParser in Sqoop shell --- .../sqoop/shell/CloneConnectionFunction.java | 10 +--------- .../apache/sqoop/shell/CloneJobFunction.java | 10 +--------- .../sqoop/shell/CreateConnectionFunction.java | 10 +--------- .../apache/sqoop/shell/CreateJobFunction.java | 15 ++------------- .../sqoop/shell/DeleteConnectionFunction.java | 10 +--------- .../apache/sqoop/shell/DeleteJobFunction.java | 10 +--------- .../sqoop/shell/DisableConnectionFunction.java | 10 +--------- .../sqoop/shell/EnableConnectionFunction.java | 10 +--------- .../apache/sqoop/shell/EnableJobFunction.java | 10 +--------- .../apache/sqoop/shell/SetOptionFunction.java | 15 ++------------- .../sqoop/shell/UpdateConnectionFunction.java | 10 +--------- .../apache/sqoop/shell/UpdateJobFunction.java | 10 +--------- .../sqoop/shell/utils/ThrowableDisplayer.java | 18 ++++++++++++------ 13 files changed, 26 insertions(+), 122 deletions(-) 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 9cce17be..d912c1ca 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java @@ -45,19 +45,11 @@ public CloneConnectionFunction() { .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 { 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 74c863d0..1b8f2b83 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 CloneJobFunction() { 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); 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 ae26035f..92a8aa5a 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/CreateConnectionFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/CreateConnectionFunction.java @@ -44,19 +44,11 @@ public CreateConnectionFunction() { 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 { 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 4ddcc12d..9c558e22 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java @@ -45,30 +45,19 @@ public CreateJobFunction() { 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 { 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 12f0083c..1eb7e51a 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java @@ -34,19 +34,11 @@ public DeleteConnectionFunction() { 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)); 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 f3ae59da..5d48c911 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/DeleteJobFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/DeleteJobFunction.java @@ -35,19 +35,11 @@ public DeleteJobFunction() { 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)); 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 6f07c321..816ff755 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java @@ -34,19 +34,11 @@ public DisableConnectionFunction() { 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); 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 9256712b..174c3df3 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/EnableConnectionFunction.java @@ -34,19 +34,11 @@ public EnableConnectionFunction() { 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); 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 e4db06ee..8575a849 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/EnableJobFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/EnableJobFunction.java @@ -35,19 +35,11 @@ public EnableJobFunction() { 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); 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 700f646a..ccc067fd 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/SetOptionFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/SetOptionFunction.java @@ -34,26 +34,15 @@ public SetOptionFunction() { 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)) { 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 8fa1bda7..405b0ea6 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/UpdateConnectionFunction.java @@ -44,19 +44,11 @@ public UpdateConnectionFunction() { 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 { 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 fbaf661f..9f5366d2 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java @@ -45,19 +45,11 @@ public UpdateJobFunction() { 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 { 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 6026a957..b9c8cad0 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()); } }