----------------------------------------------------------- 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 > >
