You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Zach Berkowitz <za...@gmail.com> on 2017/06/29 10:17:45 UTC
Review Request 60533: Make sqoop fail if user uses --direct connector
and tries to encode a null value when using a MySQL database
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60533/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-3206
https://issues.apache.org/jira/browse/SQOOP-3206
Repository: sqoop-trunk
Description
-------
Make sqoop fail if user uses --direct connector and tries to encode a null value when using a MySQL database
Diffs
-----
src/java/org/apache/sqoop/tool/ExportTool.java 5512fa7ce8a64df73a3b3d2f09fd35c5d3837489
src/java/org/apache/sqoop/tool/ImportTool.java 4b1b12d220f1612f5b473045e324491b80b50778
src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java 9fa88168f529991d98d0d64b5bf44ec294228ab1
src/test/com/cloudera/sqoop/manager/DirectMySQLTest.java a58fa1709c86560070977f495a3c51a7424b73f6
src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java dfe19529f9b81bfac1bf9af02dcc517c51f1f2fd
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java acf4fcf57989c5cfc77ff6b7fff1a5d7cea9c305
Diff: https://reviews.apache.org/r/60533/diff/1/
Testing
-------
Ran unit tests and third party tests. Unit Tests are successful, some third party tests failed for seemingly environmental reasons.
Thanks,
Zach Berkowitz
Re: Review Request 60533: Make sqoop fail if user uses --direct
connector and tries to encode a null value when using a MySQL database
Posted by Zach Berkowitz <za...@gmail.com>.
> On June 30, 2017, 9:29 a.m., Boglarka Egyed wrote:
> > src/test/org/apache/sqoop/tool/TestValidateImportOptions.java
> > Line 47 (original)
> > <https://reviews.apache.org/r/60533/diff/1/?file=1766901#file1766901line47>
> >
> > Could you please explain why have you removed this line from here?
<p>The validateDirectMysqlOptions method was expanded to include a second check on the options object. The verifyNoMoreInteractions call caused the test to fail because more methods were called on the options object than expected.</p>
- Zach
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60533/#review179342
-----------------------------------------------------------
On June 29, 2017, 10:17 a.m., Zach Berkowitz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60533/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 10:17 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3206
> https://issues.apache.org/jira/browse/SQOOP-3206
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Make sqoop fail if user uses --direct connector and tries to encode a null value when using a MySQL database
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ExportTool.java 5512fa7ce8a64df73a3b3d2f09fd35c5d3837489
> src/java/org/apache/sqoop/tool/ImportTool.java 4b1b12d220f1612f5b473045e324491b80b50778
> src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java 9fa88168f529991d98d0d64b5bf44ec294228ab1
> src/test/com/cloudera/sqoop/manager/DirectMySQLTest.java a58fa1709c86560070977f495a3c51a7424b73f6
> src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java dfe19529f9b81bfac1bf9af02dcc517c51f1f2fd
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java acf4fcf57989c5cfc77ff6b7fff1a5d7cea9c305
>
>
> Diff: https://reviews.apache.org/r/60533/diff/1/
>
>
> Testing
> -------
>
> Ran unit tests and third party tests. Unit Tests are successful, some third party tests failed for seemingly environmental reasons.
>
>
> Thanks,
>
> Zach Berkowitz
>
>
Re: Review Request 60533: Make sqoop fail if user uses --direct
connector and tries to encode a null value when using a MySQL database
Posted by Boglarka Egyed <eg...@gmail.com>.
> On June 30, 2017, 9:29 a.m., Boglarka Egyed wrote:
> > src/test/org/apache/sqoop/tool/TestValidateImportOptions.java
> > Line 47 (original)
> > <https://reviews.apache.org/r/60533/diff/1/?file=1766901#file1766901line47>
> >
> > Could you please explain why have you removed this line from here?
>
> Zach Berkowitz wrote:
> <p>The validateDirectMysqlOptions method was expanded to include a second check on the options object. The verifyNoMoreInteractions call caused the test to fail because more methods were called on the options object than expected.</p>
I see, thanks for the explanation!
- Boglarka
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60533/#review179342
-----------------------------------------------------------
On June 29, 2017, 10:17 a.m., Zach Berkowitz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60533/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 10:17 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3206
> https://issues.apache.org/jira/browse/SQOOP-3206
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Make sqoop fail if user uses --direct connector and tries to encode a null value when using a MySQL database
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ExportTool.java 5512fa7ce8a64df73a3b3d2f09fd35c5d3837489
> src/java/org/apache/sqoop/tool/ImportTool.java 4b1b12d220f1612f5b473045e324491b80b50778
> src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java 9fa88168f529991d98d0d64b5bf44ec294228ab1
> src/test/com/cloudera/sqoop/manager/DirectMySQLTest.java a58fa1709c86560070977f495a3c51a7424b73f6
> src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java dfe19529f9b81bfac1bf9af02dcc517c51f1f2fd
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java acf4fcf57989c5cfc77ff6b7fff1a5d7cea9c305
>
>
> Diff: https://reviews.apache.org/r/60533/diff/1/
>
>
> Testing
> -------
>
> Ran unit tests and third party tests. Unit Tests are successful, some third party tests failed for seemingly environmental reasons.
>
>
> Thanks,
>
> Zach Berkowitz
>
>
Re: Review Request 60533: Make sqoop fail if user uses --direct
connector and tries to encode a null value when using a MySQL database
Posted by Boglarka Egyed <eg...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60533/#review179342
-----------------------------------------------------------
Fix it, then Ship it!
Hi Zach,
Thanks for your contribution!
I have applied your patch and ran unit and 3rd party tests successfully.
Please find my minor finding below.
Thanks,
Bogi
src/test/org/apache/sqoop/tool/TestValidateImportOptions.java
Line 47 (original)
<https://reviews.apache.org/r/60533/#comment253999>
Could you please explain why have you removed this line from here?
- Boglarka Egyed
On June 29, 2017, 10:17 a.m., Zach Berkowitz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60533/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 10:17 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3206
> https://issues.apache.org/jira/browse/SQOOP-3206
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Make sqoop fail if user uses --direct connector and tries to encode a null value when using a MySQL database
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ExportTool.java 5512fa7ce8a64df73a3b3d2f09fd35c5d3837489
> src/java/org/apache/sqoop/tool/ImportTool.java 4b1b12d220f1612f5b473045e324491b80b50778
> src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java 9fa88168f529991d98d0d64b5bf44ec294228ab1
> src/test/com/cloudera/sqoop/manager/DirectMySQLTest.java a58fa1709c86560070977f495a3c51a7424b73f6
> src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java dfe19529f9b81bfac1bf9af02dcc517c51f1f2fd
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java acf4fcf57989c5cfc77ff6b7fff1a5d7cea9c305
>
>
> Diff: https://reviews.apache.org/r/60533/diff/1/
>
>
> Testing
> -------
>
> Ran unit tests and third party tests. Unit Tests are successful, some third party tests failed for seemingly environmental reasons.
>
>
> Thanks,
>
> Zach Berkowitz
>
>
Re: Review Request 60533: Make sqoop fail if user uses --direct
connector and tries to encode a null value when using a MySQL database
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60533/#review179270
-----------------------------------------------------------
Ship it!
Hi Zach,
Thank you for submitting this patch! I have ran unit and third party tests on my side and all of them was successful.
Regards,
Szabolcs
- Szabolcs Vasas
On June 29, 2017, 10:17 a.m., Zach Berkowitz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60533/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 10:17 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3206
> https://issues.apache.org/jira/browse/SQOOP-3206
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Make sqoop fail if user uses --direct connector and tries to encode a null value when using a MySQL database
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ExportTool.java 5512fa7ce8a64df73a3b3d2f09fd35c5d3837489
> src/java/org/apache/sqoop/tool/ImportTool.java 4b1b12d220f1612f5b473045e324491b80b50778
> src/test/com/cloudera/sqoop/manager/DirectMySQLExportTest.java 9fa88168f529991d98d0d64b5bf44ec294228ab1
> src/test/com/cloudera/sqoop/manager/DirectMySQLTest.java a58fa1709c86560070977f495a3c51a7424b73f6
> src/test/org/apache/sqoop/tool/TestExportToolValidateOptions.java dfe19529f9b81bfac1bf9af02dcc517c51f1f2fd
> src/test/org/apache/sqoop/tool/TestValidateImportOptions.java acf4fcf57989c5cfc77ff6b7fff1a5d7cea9c305
>
>
> Diff: https://reviews.apache.org/r/60533/diff/1/
>
>
> Testing
> -------
>
> Ran unit tests and third party tests. Unit Tests are successful, some third party tests failed for seemingly environmental reasons.
>
>
> Thanks,
>
> Zach Berkowitz
>
>