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
Hari Shreedharan 2013-08-02, 06:02

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13181/#review24508
-----------------------------------------------------------
Abe,

Thanks for the patch. This looks pretty good. I did a quick initial and partial review - so I could get some feedback since I'll be able to get back to this only on Monday. Please note that most of the comments follow a pattern - so you should be able to fix it in more classes than I have mentioned here.
client/src/main/java/org/apache/sqoop/client/shell/CloneConnectionFunction.java
<https://reviews.apache.org/r/13181/#comment48487>

    Shouldn't isInteractive arg with value false be passed in?

client/src/main/java/org/apache/sqoop/client/shell/CloneJobFunction.java
<https://reviews.apache.org/r/13181/#comment48488>

    isInteractive = false to be passed in?

client/src/main/java/org/apache/sqoop/client/shell/CloneJobFunction.java
<https://reviews.apache.org/r/13181/#comment48489>

    Ideally, for 2 methods with the same name, you should not have them behave differently - one is setting isInteractive = false and the other sets it to true.

client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java
<https://reviews.apache.org/r/13181/#comment48490>

    Same as above, they should behave in the same way

client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java
<https://reviews.apache.org/r/13181/#comment48491>

    Same as above

client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java
<https://reviews.apache.org/r/13181/#comment48492>

    This same method seems to be overridden in all subclasses - is it possible to move this up, and just pass in the options which should be handled?
- Hari Shreedharan
On Aug. 1, 2013, 12:40 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13181/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2013, 12:40 a.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 d894ac7b09a18f3fed1fb58bed7554a873fa8630
> Author: Abraham Elmahrek <[EMAIL PROTECTED]>
> Date:   Wed Jul 31 14:23:58 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 0538901... 7f5df34... M client/src/main/java/org/apache/sqoop/client/shell/CloneConnectionFunction.java
> :100644 100644 6f62813... 32f8c3f... M client/src/main/java/org/apache/sqoop/client/shell/CloneJobFunction.java
> :100644 100644 ac555e1... c842c4d... M client/src/main/java/org/apache/sqoop/client/shell/CreateCommand.java
> :100644 100644 04b240c... f23e479... M client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java
> :100644 100644 cc4d546... 7b40645... M client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java
> :100644 100644 18d3a70... 5220d61... M client/src/main/java/org/apache/sqoop/client/shell/DeleteConnectionFunction.java
> :100644 100644 736be20... e82a47b... M client/src/main/java/org/apache/sqoop/client/shell/DeleteJobFunction.java
> :100644 100644 e04292a... f6cd6e3... M client/src/main/java/org/apache/sqoop/client/shell/DisableConnectionFunction.java
> :100644 100644 5962cd2... 6ea9f0c... M client/src/main/java/org/apache/sqoop/client/shell/DisableJobFunction.java