----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1946/#review2132 -----------------------------------------------------------
Ship it! Changes look good Jarek. There are a couple of checkstyle violations noted below. Other than that, it is good to go. Please address these checkstyle violations and attach the patch to the JIRA. /src/java/com/cloudera/sqoop/mapreduce/db/DataDrivenDBInputFormat.java <https://reviews.apache.org/r/1946/#comment4951> Checkstyle violation: ')' is preceded with whitespace. /src/test/com/cloudera/sqoop/TestBoundaryQuery.java <https://reviews.apache.org/r/1946/#comment4952> Checkstyle violation: trailing whitespace. - Arvind On 2011-09-28 14:21:32, Jarek Jarcec wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1946/ > ----------------------------------------------------------- > > (Updated 2011-09-28 14:21:32) > > > Review request for Sqoop and Arvind Prabhakar. > > > Summary > ------- > > I've incorporated all Arvind's suggestions (hopefully :-)). > > > This addresses bug SQOOP-331. > https://issues.apache.org/jira/browse/SQOOP-331 > > > Diffs > ----- > > /src/docs/man/import-args.txt 1176793 > /src/docs/user/import.txt 1176793 > /src/java/com/cloudera/sqoop/SqoopOptions.java 1176793 > /src/java/com/cloudera/sqoop/manager/SqlManager.java 1176793 > /src/java/com/cloudera/sqoop/mapreduce/DataDrivenImportJob.java 1176793 > /src/java/com/cloudera/sqoop/mapreduce/db/DataDrivenDBInputFormat.java > 1176793 > /src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java 1176793 > /src/java/com/cloudera/sqoop/tool/ImportTool.java 1176793 > /src/test/com/cloudera/sqoop/TestBoundaryQuery.java PRE-CREATION > /src/test/com/cloudera/sqoop/TestSqoopOptions.java 1176793 > > Diff: https://reviews.apache.org/r/1946/diff > > > Testing > ------- > > I'm still having troubles to create meaningful tests for this patch. I've > came up with two different approaches, but I wasn't able to get running > either of them: > > 1) Use boundary query for limiting import data (like "select 1, 2"). This is > totally wrong usage of this parameter, but I was thinking that It might be > fine for the testing purpose. Unfortunately underlying code is using this > query only in case that is creating more than one map task and I was not able > to forced it create more than one. Which make sense because the -m parameter > is also only a hint. > > 2) Parse logs. Fortunately class responsible for creating splits is printing > used boundary query, so there is possibility to parse those logs and look for > used boundary query. But I'm not sure how this can be done in proper fashion. > > Any ideas will be welcomed. > > Jarcec > > > Thanks, > > Jarek > >
