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


Hi Mengwei,
thank you very much for working on this JIRA!

I would advise to stay consistent with other commands and keep the distinction 
between "command" and "function", eg. something like "start job", "stop job" 
and "status job". I know that we currently can't start anything else beyond 
job, but it might be interesting to keep that option for the future.


client/src/main/java/org/apache/sqoop/client/shell/StartCommand.java
<https://reviews.apache.org/r/11983/#comment45772>

    I think that this if is missing:
    
    ... && has_option(jid)



client/src/main/java/org/apache/sqoop/client/shell/StartCommand.java
<https://reviews.apache.org/r/11983/#comment45773>

    I would suggest to propagate the InterruptedException up. It will be 
eventually caught up by the shell and based on the user configuration printed 
with or without details (stack trace).



client/src/main/java/org/apache/sqoop/client/utils/OptionParser.java
<https://reviews.apache.org/r/11983/#comment45774>

    I do not feel that this refactoring is providing a huge value here. All the 
methods were already present only once in parent implementation of all commands 
and thus were reused.


Jarcec

- Jarek Cecho


On June 21, 2013, 5:51 p.m., Mengwei Ding wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11983/
> -----------------------------------------------------------
> 
> (Updated June 21, 2013, 5:51 p.m.)
> 
> 
> Review request for Sqoop, Jarek Cecho, Hari Shreedharan, and Abraham Elmahrek.
> 
> 
> Description
> -------
> 
> commit 5cc954a0e8594a1ddd667d06cc438100b44fe234
> Author: Mengwei Ding <[email protected]>
> Date:   Wed Jun 19 17:56:57 2013 -0700
> 
>     SQOOP-675 replace 'submission' cmd with 'start', 'stop' and 'status' cmds.
> 
> :100644 100644 056fcc8... 1020209... M        
> client/src/main/java/org/apache/sqoop/client/core/Constants.java
> :100644 100644 0538901... 519e571... M        
> client/src/main/java/org/apache/sqoop/client/shell/CloneConnectionFunction.java
> :100644 100644 6f62813... fce737c... M        
> client/src/main/java/org/apache/sqoop/client/shell/CloneJobFunction.java
> :100644 100644 04b240c... 7729363... M        
> client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java
> :100644 100644 cc4d546... fc6f4ff... M        
> client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java
> :100644 100644 18d3a70... c5b7911... M        
> client/src/main/java/org/apache/sqoop/client/shell/DeleteConnectionFunction.java
> :100644 100644 736be20... 71553cd... M        
> client/src/main/java/org/apache/sqoop/client/shell/DeleteJobFunction.java
> :100644 100644 32bca71... 3c9039a... M        
> client/src/main/java/org/apache/sqoop/client/shell/ShowConnectionFunction.java
> :100644 100644 b053339... 81a78b0... M        
> client/src/main/java/org/apache/sqoop/client/shell/ShowConnectorFunction.java
> :100644 100644 590e4e7... 4f9dfb4... M        
> client/src/main/java/org/apache/sqoop/client/shell/ShowJobFunction.java
> :100644 100644 bf26761... b148667... M        
> client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java
> :100644 100644 83f1c4f... aa87f20... M        
> client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java
> :000000 100644 0000000... c076882... A        
> client/src/main/java/org/apache/sqoop/client/shell/StartCommand.java
> :000000 100644 0000000... ef2cec6... A        
> client/src/main/java/org/apache/sqoop/client/shell/StatusCommand.java
> :000000 100644 0000000... f483569... A        
> client/src/main/java/org/apache/sqoop/client/shell/StopCommand.java
> :100644 000000 993bbde... 0000000... D        
> client/src/main/java/org/apache/sqoop/client/shell/SubmissionCommand.java
> :100644 000000 04bcf45... 0000000... D        
> client/src/main/java/org/apache/sqoop/client/shell/SubmissionStartFunction.java
> :100644 000000 1a6d896... 0000000... D        
> client/src/main/java/org/apache/sqoop/client/shell/SubmissionStatusFunction.java
> :100644 000000 c407d01... 0000000... D        
> client/src/main/java/org/apache/sqoop/client/shell/SubmissionStopFunction.java
> :100644 100644 8556e2b... b6b15fd... M        
> client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java
> :100644 100644 425a53f... 8aa5d62... M        
> client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java
> :000000 100644 0000000... ca6443b... A        
> client/src/main/java/org/apache/sqoop/client/utils/OptionParser.java
> :100644 100644 b159757... 2092a33... M        
> client/src/main/resources/client-resource.properties
> :100644 100644 073d547... a103277... M        
> docs/src/site/sphinx/CommandLineClient.rst
> 
> 
> This addresses bug SQOOP-675.
>     https://issues.apache.org/jira/browse/SQOOP-675
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/sqoop/client/core/Constants.java 056fcc8 
>   
> client/src/main/java/org/apache/sqoop/client/shell/CloneConnectionFunction.java
>  0538901 
>   client/src/main/java/org/apache/sqoop/client/shell/CloneJobFunction.java 
> 6f62813 
>   
> client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java
>  04b240c 
>   client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java 
> cc4d546 
>   
> client/src/main/java/org/apache/sqoop/client/shell/DeleteConnectionFunction.java
>  18d3a70 
>   client/src/main/java/org/apache/sqoop/client/shell/DeleteJobFunction.java 
> 736be20 
>   
> client/src/main/java/org/apache/sqoop/client/shell/ShowConnectionFunction.java
>  32bca71 
>   
> client/src/main/java/org/apache/sqoop/client/shell/ShowConnectorFunction.java 
> b053339 
>   client/src/main/java/org/apache/sqoop/client/shell/ShowJobFunction.java 
> 590e4e7 
>   client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java 
> bf26761 
>   client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java 83f1c4f 
>   client/src/main/java/org/apache/sqoop/client/shell/StartCommand.java 
> PRE-CREATION 
>   client/src/main/java/org/apache/sqoop/client/shell/StatusCommand.java 
> PRE-CREATION 
>   client/src/main/java/org/apache/sqoop/client/shell/StopCommand.java 
> PRE-CREATION 
>   client/src/main/java/org/apache/sqoop/client/shell/SubmissionCommand.java 
> 993bbde 
>   
> client/src/main/java/org/apache/sqoop/client/shell/SubmissionStartFunction.java
>  04bcf45 
>   
> client/src/main/java/org/apache/sqoop/client/shell/SubmissionStatusFunction.java
>  1a6d896 
>   
> client/src/main/java/org/apache/sqoop/client/shell/SubmissionStopFunction.java
>  c407d01 
>   
> client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java
>  8556e2b 
>   client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java 
> 425a53f 
>   client/src/main/java/org/apache/sqoop/client/utils/OptionParser.java 
> PRE-CREATION 
>   client/src/main/resources/client-resource.properties b159757 
>   docs/src/site/sphinx/CommandLineClient.rst 073d547 
> 
> Diff: https://reviews.apache.org/r/11983/diff/
> 
> 
> Testing
> -------
> 
> Done several manual test for all these 3 commands and their options.
> 
> 
> Thanks,
> 
> Mengwei Ding
> 
>

Reply via email to