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


Thanks for the patch Dian. Overall it looks good to me. I was playing with the 
new command line a bit and I have several observations - I'm wondering if you 
seen them as well?

1) When updating entities (update link -l "My link") I'm not given previous 
content of the field, I'm for example given empty name:

  Name:
  
However when I press any key the content magically appears.

2) Pressing backspace removes several characters at once. For example consider 
line with "Name: My MySQL", if I press backspace the line converts to "Name: M"
 
3) I'm not sure whether it's a new issue or not, but if I enter invalid name, 
I'm only given error but not the ability to change the name, check it out:

  sqoop:000> create link -c generic-jdbc-connector
  Creating link for connector with id generic-jdbc-connector
  Please fill following values to create new link object
  Name:
  object-name: Error message: Job name or link name cannot be null

  Link configuration

Shouldn't we make a loop and request valid Name rather then just typing the 
error message?

Jarcec

- Jarek Cecho


On Nov. 16, 2015, 11:45 a.m., Dian Fu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40281/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2015, 11:45 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2684
>     https://issues.apache.org/jira/browse/SQOOP-2684
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> Currently groovy version used in sqoop is 1.8.5. This version doesn't support 
> space in command arguments(GROOVY-6942). This make it impossible to run 
> commands like this start job "job name with whitespace". I'd like to propose 
> to upgrade the groovy version to 2.4.0.
> 
> 
> Diffs
> -----
> 
>   pom.xml f33958c 
>   shell/pom.xml c70c745 
>   shell/src/main/java/org/apache/sqoop/shell/CloneCommand.java f71f82c 
>   shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java ecea579 
>   shell/src/main/java/org/apache/sqoop/shell/CloneLinkFunction.java b46a19f 
>   shell/src/main/java/org/apache/sqoop/shell/CreateCommand.java a16ba5b 
>   shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java 03ceaba 
>   shell/src/main/java/org/apache/sqoop/shell/CreateLinkFunction.java e392846 
>   shell/src/main/java/org/apache/sqoop/shell/DeleteCommand.java 4b66505 
>   shell/src/main/java/org/apache/sqoop/shell/DisableCommand.java 03aa922 
>   shell/src/main/java/org/apache/sqoop/shell/EnableCommand.java 42cc9fc 
>   shell/src/main/java/org/apache/sqoop/shell/GrantCommand.java b4b77b0 
>   shell/src/main/java/org/apache/sqoop/shell/HelpCommand.java e8d531f 
>   shell/src/main/java/org/apache/sqoop/shell/RevokeCommand.java 0cb30e4 
>   shell/src/main/java/org/apache/sqoop/shell/SetCommand.java 3b8f4c2 
>   shell/src/main/java/org/apache/sqoop/shell/ShellEnvironment.java 55a0f27 
>   shell/src/main/java/org/apache/sqoop/shell/ShowCommand.java 82c52da 
>   shell/src/main/java/org/apache/sqoop/shell/SqoopCommand.java fdb7a5b 
>   shell/src/main/java/org/apache/sqoop/shell/SqoopShell.java 33fddbf 
>   shell/src/main/java/org/apache/sqoop/shell/StartCommand.java 7c56980 
>   shell/src/main/java/org/apache/sqoop/shell/StatusCommand.java 5b4ef1f 
>   shell/src/main/java/org/apache/sqoop/shell/StopCommand.java 50b2e81 
>   shell/src/main/java/org/apache/sqoop/shell/UpdateCommand.java c75d5f5 
>   shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java fe6a155 
>   shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java c5359ce 
>   shell/src/main/java/org/apache/sqoop/shell/utils/ConfigFiller.java 3a4a18d 
>   shell/src/test/java/org/apache/sqoop/shell/TestCloneCommand.java c45501c 
>   shell/src/test/java/org/apache/sqoop/shell/TestCreateCommand.java 2f91897 
>   shell/src/test/java/org/apache/sqoop/shell/TestUpdateCommand.java d5df497 
>   shell/src/test/java/org/apache/sqoop/shell/utils/TestConfigFiller.java 
> 1917a1d 
> 
> Diff: https://reviews.apache.org/r/40281/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dian Fu
> 
>

Reply via email to