You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Anna Szonyi <sz...@cloudera.com> on 2016/08/12 08:28:45 UTC
Review Request 50987: SQOOP-2913
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-2913
https://issues.apache.org/jira/browse/SQOOP-2913
Repository: sqoop-trunk
Description
-------
SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
Diffs
-----
src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
Diff: https://reviews.apache.org/r/50987/diff/
Testing
-------
Added new test cases
Thanks,
Anna Szonyi
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/#review145612
-----------------------------------------------------------
src/java/org/apache/sqoop/tool/ImportTool.java (line 1)
<https://reviews.apache.org/r/50987/#comment211916>
It seems the whole file was reformatted and because of this every line seems to be changed. Can we separate the reformatting to another patch if it is necessary?
- Szabolcs Vasas
On Aug. 12, 2016, 8:31 a.m., Anna Szonyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50987/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2016, 8:31 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2913
> https://issues.apache.org/jira/browse/SQOOP-2913
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
> src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
> src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
> src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
> src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50987/diff/
>
>
> Testing
> -------
>
> Added new test cases
>
>
> Thanks,
>
> Anna Szonyi
>
>
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Boglarka Egyed <eg...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/#review145606
-----------------------------------------------------------
Fix it, then Ship it!
src/java/org/apache/sqoop/manager/DefaultManagerFactory.java (line 28)
<https://reviews.apache.org/r/50987/#comment211912>
Imports with * option should be avoided according to the community's coding guideline. Sqoop2 coding guideline can be found here: https://cwiki.apache.org/confluence/display/SQOOP/Sqoop+2+Coding+Guidelines which can be considered as Sqoop1 coding guideline as well.
src/java/org/apache/sqoop/manager/Manager.java (lines 7 - 9)
<https://reviews.apache.org/r/50987/#comment211914>
In case of new file creation every file shall contain a head Apache license comment at the beginning of the file. Please check other files for reference, also, you can set a configuration in your IDEA to do it automatically during every file creation.
src/test/com/cloudera/sqoop/TestDirectImport.java (lines 14 - 16)
<https://reviews.apache.org/r/50987/#comment211917>
Please change this to Apache license comment also.
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java (line 12)
<https://reviews.apache.org/r/50987/#comment211918>
Please don't use wildcard imports regarding the community's coding guideline.
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java (lines 14 - 16)
<https://reviews.apache.org/r/50987/#comment211919>
Please include Apache license comment instead.
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java (line 9)
<https://reviews.apache.org/r/50987/#comment211921>
Please update this, also.
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java (lines 11 - 13)
<https://reviews.apache.org/r/50987/#comment211920>
Change this also, please.
- Boglarka Egyed
On Aug. 12, 2016, 8:31 a.m., Anna Szonyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50987/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2016, 8:31 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2913
> https://issues.apache.org/jira/browse/SQOOP-2913
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
> src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
> src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
> src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
> src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50987/diff/
>
>
> Testing
> -------
>
> Added new test cases
>
>
> Thanks,
>
> Anna Szonyi
>
>
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Attila Szabo <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/#review150179
-----------------------------------------------------------
Ship it!
Ship It!
- Attila Szabo
On Sept. 23, 2016, 2:09 p.m., Anna Szonyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50987/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2016, 2:09 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2913
> https://issues.apache.org/jira/browse/SQOOP-2913
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7
> src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
> src/java/org/apache/sqoop/manager/SupportedManagers.java PRE-CREATION
> src/java/org/apache/sqoop/tool/ExportTool.java 15ecddf
> src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
> src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
> src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50987/diff/
>
>
> Testing
> -------
>
> Added new test cases
>
>
> Thanks,
>
> Anna Szonyi
>
>
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/
-----------------------------------------------------------
(Updated Sept. 23, 2016, 2:09 p.m.)
Review request for Sqoop.
Changes
-------
Renamed Manager to SupportManagers
Bugs: SQOOP-2913
https://issues.apache.org/jira/browse/SQOOP-2913
Repository: sqoop-trunk
Description
-------
SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
Diffs (updated)
-----
src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7
src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
src/java/org/apache/sqoop/manager/SupportedManagers.java PRE-CREATION
src/java/org/apache/sqoop/tool/ExportTool.java 15ecddf
src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
Diff: https://reviews.apache.org/r/50987/diff/
Testing
-------
Added new test cases
Thanks,
Anna Szonyi
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Erzsebet Szilagyi <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/#review146639
-----------------------------------------------------------
Ship it!
Ship It!
- Erzsebet Szilagyi
On Aug. 23, 2016, 4:11 p.m., Anna Szonyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50987/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2016, 4:11 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2913
> https://issues.apache.org/jira/browse/SQOOP-2913
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7
> src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
> src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
> src/java/org/apache/sqoop/tool/ExportTool.java 15ecddf
> src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
> src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
> src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50987/diff/
>
>
> Testing
> -------
>
> Added new test cases
>
>
> Thanks,
>
> Anna Szonyi
>
>
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Attila Szabo <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/#review150175
-----------------------------------------------------------
Hey Anna,
The final implementation looks great.
I have only one naming convention related request for you:
Could you please give a much more concrete name to your Manager enum? Like SupportedManagers, SuppoertedConnManagers, etc.
Like this it's too abstract and could be confusing.
Thanks,
Maugli
- Attila Szabo
On Aug. 23, 2016, 2:11 p.m., Anna Szonyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50987/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2016, 2:11 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2913
> https://issues.apache.org/jira/browse/SQOOP-2913
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7
> src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
> src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
> src/java/org/apache/sqoop/tool/ExportTool.java 15ecddf
> src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
> src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
> src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50987/diff/
>
>
> Testing
> -------
>
> Added new test cases
>
>
> Thanks,
>
> Anna Szonyi
>
>
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/
-----------------------------------------------------------
(Updated Aug. 23, 2016, 2:11 p.m.)
Review request for Sqoop.
Changes
-------
Fixed typo highlighted by reviewer
Bugs: SQOOP-2913
https://issues.apache.org/jira/browse/SQOOP-2913
Repository: sqoop-trunk
Description
-------
SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
Diffs (updated)
-----
src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7
src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
src/java/org/apache/sqoop/tool/ExportTool.java 15ecddf
src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
Diff: https://reviews.apache.org/r/50987/diff/
Testing
-------
Added new test cases
Thanks,
Anna Szonyi
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/#review146319
-----------------------------------------------------------
src/java/org/apache/sqoop/tool/ImportTool.java (line 1110)
<https://reviews.apache.org/r/50987/#comment212697>
There is a typo in the method name, the t is missing from direct.
- Szabolcs Vasas
On Aug. 19, 2016, 5:15 p.m., Anna Szonyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50987/
> -----------------------------------------------------------
>
> (Updated Aug. 19, 2016, 5:15 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2913
> https://issues.apache.org/jira/browse/SQOOP-2913
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7
> src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
> src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
> src/java/org/apache/sqoop/tool/ExportTool.java 15ecddf
> src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
> src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
> src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java PRE-CREATION
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/50987/diff/
>
>
> Testing
> -------
>
> Added new test cases
>
>
> Thanks,
>
> Anna Szonyi
>
>
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/
-----------------------------------------------------------
(Updated Aug. 19, 2016, 5:15 p.m.)
Review request for Sqoop.
Changes
-------
Added validation and test for Export tool (checked that JobTool automatically calls the "childTool" validation as well)
Bugs: SQOOP-2913
https://issues.apache.org/jira/browse/SQOOP-2913
Repository: sqoop-trunk
Description
-------
SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
Diffs (updated)
-----
src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7
src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
src/java/org/apache/sqoop/tool/ExportTool.java 15ecddf
src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
Diff: https://reviews.apache.org/r/50987/diff/
Testing
-------
Added new test cases
Thanks,
Anna Szonyi
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/
-----------------------------------------------------------
(Updated Aug. 15, 2016, 8:04 a.m.)
Review request for Sqoop.
Changes
-------
TestDirect import changes: force JUnit4, so we can assert on the exception in a cleaner fashion
Bugs: SQOOP-2913
https://issues.apache.org/jira/browse/SQOOP-2913
Repository: sqoop-trunk
Description
-------
SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
Diff: https://reviews.apache.org/r/50987/diff/
Testing
-------
Added new test cases
Thanks,
Anna Szonyi
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/
-----------------------------------------------------------
(Updated Aug. 12, 2016, 4:21 p.m.)
Review request for Sqoop.
Changes
-------
Incorporated the changes suggested by the reviewers.
Bugs: SQOOP-2913
https://issues.apache.org/jira/browse/SQOOP-2913
Repository: sqoop-trunk
Description
-------
SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
Diff: https://reviews.apache.org/r/50987/diff/
Testing
-------
Added new test cases
Thanks,
Anna Szonyi
Re: Review Request 50987: SQOOP-2913: Make sqoop fail if user uses
--direct connector for case when --direct connector is not available
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50987/
-----------------------------------------------------------
(Updated Aug. 12, 2016, 8:31 a.m.)
Review request for Sqoop.
Summary (updated)
-----------------
SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
Bugs: SQOOP-2913
https://issues.apache.org/jira/browse/SQOOP-2913
Repository: sqoop-trunk
Description
-------
SQOOP-2913: Make sqoop fail if user uses --direct connector for case when --direct connector is not available
Diffs
-----
src/java/org/apache/sqoop/manager/DefaultManagerFactory.java 58ff745
src/java/org/apache/sqoop/manager/Manager.java PRE-CREATION
src/java/org/apache/sqoop/tool/ImportTool.java ff7b822
src/test/com/cloudera/sqoop/TestDirectImport.java PRE-CREATION
src/test/org/apache/sqoop/manager/TestDefaultManagerFactory.java PRE-CREATION
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java PRE-CREATION
Diff: https://reviews.apache.org/r/50987/diff/
Testing
-------
Added new test cases
Thanks,
Anna Szonyi