You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Jarek Cecho <ja...@apache.org> on 2013/04/06 01:52:27 UTC
Review Request: SQOOP-979 MySQL direct connector is not working correct
after moving password to credential cache
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10322/
-----------------------------------------------------------
Review request for Sqoop.
Description
-------
I've changed the code to use DBConfiguration object to retrieve the password instead of directly using the old properties. I've also done two questionable changes that I would like to highlight:
1) I've removed MySQLUtils.PASSWORD_KEY. This is internal property of MySQL direct connector and should not be used in any dependent code. I would argue that by deleting it, any affected third party code will fail very quickly rather than causing random exceptions later as the property will be always empty.
2) I've change method DBConfiguration.getPassword() to public. I would argue that this method can be called only by Sqoop and connectors do have valid need to get the password so that it can be passed to third party applications like mysqldump.
This addresses bug SQOOP-979.
https://issues.apache.org/jira/browse/SQOOP-979
Diffs
-----
src/java/com/cloudera/sqoop/manager/MySQLUtils.java 6611f8eaa7027b9fc2d384c41d55f9e77fdf8e0e
src/java/org/apache/sqoop/manager/MySQLUtils.java c86cf1a6e25852764c8bb3d768441a9eb09142e3
src/java/org/apache/sqoop/mapreduce/MySQLDumpMapper.java 4daaaeb51654cc76a55636abb82c8f9d2ebb2f2e
src/java/org/apache/sqoop/mapreduce/MySQLExportMapper.java dc1c1263e5d92bca3eda3dec5a52b6e0c92284f8
src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java 4bd066db40bd1d8f5eb04fdc541e5e1a1e300a09
Diff: https://reviews.apache.org/r/10322/diff/
Testing
-------
Failing MySQLAuthTest and DirectMySQLExportTest test cases seems to be working after applying the patch.
Thanks,
Jarek Cecho
Re: Review Request: SQOOP-979 MySQL direct connector is not working correct
after moving password to credential cache
Posted by Kathleen Ting <ka...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10322/#review18755
-----------------------------------------------------------
Ship it!
Looks good. Tests passed. Thanks Jarcec.
- Kathleen Ting
On April 5, 2013, 11:52 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10322/
> -----------------------------------------------------------
>
> (Updated April 5, 2013, 11:52 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've changed the code to use DBConfiguration object to retrieve the password instead of directly using the old properties. I've also done two questionable changes that I would like to highlight:
>
> 1) I've removed MySQLUtils.PASSWORD_KEY. This is internal property of MySQL direct connector and should not be used in any dependent code. I would argue that by deleting it, any affected third party code will fail very quickly rather than causing random exceptions later as the property will be always empty.
>
> 2) I've change method DBConfiguration.getPassword() to public. I would argue that this method can be called only by Sqoop and connectors do have valid need to get the password so that it can be passed to third party applications like mysqldump.
>
>
> This addresses bug SQOOP-979.
> https://issues.apache.org/jira/browse/SQOOP-979
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/manager/MySQLUtils.java 6611f8eaa7027b9fc2d384c41d55f9e77fdf8e0e
> src/java/org/apache/sqoop/manager/MySQLUtils.java c86cf1a6e25852764c8bb3d768441a9eb09142e3
> src/java/org/apache/sqoop/mapreduce/MySQLDumpMapper.java 4daaaeb51654cc76a55636abb82c8f9d2ebb2f2e
> src/java/org/apache/sqoop/mapreduce/MySQLExportMapper.java dc1c1263e5d92bca3eda3dec5a52b6e0c92284f8
> src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java 4bd066db40bd1d8f5eb04fdc541e5e1a1e300a09
>
> Diff: https://reviews.apache.org/r/10322/diff/
>
>
> Testing
> -------
>
> Failing MySQLAuthTest and DirectMySQLExportTest test cases seems to be working after applying the patch.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-979 MySQL direct connector is not working correct
after moving password to credential cache
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10322/#review18745
-----------------------------------------------------------
Ship it!
Looks good. Good catch to identify the issue.
- Venkat Ranganathan
On April 5, 2013, 11:52 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10322/
> -----------------------------------------------------------
>
> (Updated April 5, 2013, 11:52 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've changed the code to use DBConfiguration object to retrieve the password instead of directly using the old properties. I've also done two questionable changes that I would like to highlight:
>
> 1) I've removed MySQLUtils.PASSWORD_KEY. This is internal property of MySQL direct connector and should not be used in any dependent code. I would argue that by deleting it, any affected third party code will fail very quickly rather than causing random exceptions later as the property will be always empty.
>
> 2) I've change method DBConfiguration.getPassword() to public. I would argue that this method can be called only by Sqoop and connectors do have valid need to get the password so that it can be passed to third party applications like mysqldump.
>
>
> This addresses bug SQOOP-979.
> https://issues.apache.org/jira/browse/SQOOP-979
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/manager/MySQLUtils.java 6611f8eaa7027b9fc2d384c41d55f9e77fdf8e0e
> src/java/org/apache/sqoop/manager/MySQLUtils.java c86cf1a6e25852764c8bb3d768441a9eb09142e3
> src/java/org/apache/sqoop/mapreduce/MySQLDumpMapper.java 4daaaeb51654cc76a55636abb82c8f9d2ebb2f2e
> src/java/org/apache/sqoop/mapreduce/MySQLExportMapper.java dc1c1263e5d92bca3eda3dec5a52b6e0c92284f8
> src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java 4bd066db40bd1d8f5eb04fdc541e5e1a1e300a09
>
> Diff: https://reviews.apache.org/r/10322/diff/
>
>
> Testing
> -------
>
> Failing MySQLAuthTest and DirectMySQLExportTest test cases seems to be working after applying the patch.
>
>
> Thanks,
>
> Jarek Cecho
>
>