You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Dian Fu <di...@gmail.com> on 2016/01/04 02:48:57 UTC

Re: Review Request 41827: Sqoop2: Sqoop shell is accepting non existing arguments


> On Dec. 31, 2015, 2:13 p.m., Jarek Cecho wrote:
> > Looks good to me. Precommit hook complained about failing unit test TestShowCommand. Can you look into why it's failing?
> > 
> > Otherwise I'm +1.

Hi Jarcec,
Thanks a lot for the review. Updated the patch.


- Dian


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


On Dec. 31, 2015, 7:43 a.m., Dian Fu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41827/
> -----------------------------------------------------------
> 
> (Updated Dec. 31, 2015, 7:43 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2763
>     https://issues.apache.org/jira/browse/SQOOP-2763
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> We happily accept non-existing arguments in the shell which is highly deceptive:
> sqoop:000> set server --name x
> Server is set successfully
> sqoop:000>
> or
> sqoop:000> show connector --please
> ...
> sqoop:000>
> We should throw an exception or something if one is using incorrect/non-existing arguments.
> 
> 
> Diffs
> -----
> 
>   common/pom.xml 078a785 
>   common/src/main/java/org/apache/sqoop/cli/SqoopGnuParser.java PRE-CREATION 
>   shell/pom.xml 1949a8e 
>   shell/src/main/java/org/apache/sqoop/shell/SqoopFunction.java 0845d8e 
>   shell/src/main/java/org/apache/sqoop/shell/utils/ConfigOptions.java 97b6e3b 
>   tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryDumpTool.java 6958ee1 
>   tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java 748ef80 
> 
> Diff: https://reviews.apache.org/r/41827/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dian Fu
> 
>