Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Sqoop >> mail # dev >> Review Request 13181: SQOOP-773 Sqoop2: Batch execution support for client commands


Copy link to this message
-
Re: Review Request 13181: SQOOP-773 Sqoop2: Batch execution support for client commands

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13181/#review26356
-----------------------------------------------------------
Hi Abe,
thank you very much for working on this patch, greatly appreciated! I've took a look and I do have couple of notes:
shell/src/main/java/org/apache/sqoop/shell/CreateCommand.java
<https://reviews.apache.org/r/13181/#comment51475>

    Would it make sense to remove the constant SHELL_0007 when it's unused now?

shell/src/main/java/org/apache/sqoop/shell/SqoopFunction.java
<https://reviews.apache.org/r/13181/#comment51477>

    It seems that majority of the downstream implementations ends up calling the same method with boolean isInteractive as parameter, so I'm wondering if it would make sense to make it simpler and have executeFunction(CommandLine, Boolean isInteractive) in the first place. What do you think?

shell/src/main/java/org/apache/sqoop/shell/SqoopFunction.java
<https://reviews.apache.org/r/13181/#comment51444>

    Considering that almost all execute*Function() will have to validate the arguments the same way, do you think that it would make sense to introduce a new abstract method validateCommandLine(CommandLine, interactive)?

shell/src/main/java/org/apache/sqoop/shell/utils/FormFiller.java
<https://reviews.apache.org/r/13181/#comment51478>

    I would suggest to use the value rather than index in case of batch loading of Enum values.

shell/src/main/java/org/apache/sqoop/shell/utils/FormOptions.java
<https://reviews.apache.org/r/13181/#comment51476>

    I'm wondering if something like srglist.subList(start, arglist.size()).toArray() wouldn't be simpler here?
Jarcec

- Jarek Cecho
On Sept. 23, 2013, 11:48 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13181/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2013, 11:48 p.m.)
>
>
> Review request for Sqoop, Hari Shreedharan and Jarek Cecho.
>
>
> Bugs: SQOOP-773
>     https://issues.apache.org/jira/browse/SQOOP-773
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 080ced16578c1d95015ce3e99b4335beb465861a
> Author: Abraham Elmahrek <[EMAIL PROTECTED]>
> Date:   Tue Aug 13 14:14:06 2013 -0700
>
>     SQOOP-773 Sqoop2: Batch execution support for client commands
>    
>     Separated options into two groups: fixed and dynamic options.
>     Fixed options (IE: connector ID) come first and are used to select
>     what options should be used in dynamic options. Dynamic options
>     are automatically created based on forms selected from fixed options.
>     The keys for these options take on the form "<prefix>-<form name>-<input-name>".
>
> :100644 100644 980a908... a7e7e7d... M  shell/src/main/java/org/apache/sqoop/shell/CloneCommand.java
> :100644 100644 856abaa... 2c2869c... M  shell/src/main/java/org/apache/sqoop/shell/CloneConnectionFunction.java
> :100644 100644 3e23025... dd2eb2b... M  shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java
> :100644 100644 e62ce08... 9ad007b... M  shell/src/main/java/org/apache/sqoop/shell/CreateCommand.java
> :100644 100644 5fbf0a3... 973fd53... M  shell/src/main/java/org/apache/sqoop/shell/CreateConnectionFunction.java
> :100644 100644 6e4f04b... f0d4a6c... M  shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java
> :100644 100644 c123732... d79516d... M  shell/src/main/java/org/apache/sqoop/shell/DeleteConnectionFunction.java
> :100644 100644 d4095b7... df9a2cc... M  shell/src/main/java/org/apache/sqoop/shell/DeleteJobFunction.java
> :100644 100644 f119660... b5a54b9... M  shell/src/main/java/org/apache/sqoop/shell/DisableConnectionFunction.java
> :100644 100644 a87e51f... 8cc4ea3... M  shell/src/main/java/org/apache/sqoop/shell/DisableJobFunction.java