----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31839/#review75902 -----------------------------------------------------------
Overall looks good to me. I have one suggestion though - can we add test that will have two tables such that: 1) First to import doesn't have PK and will use the new parameter to get to 1 mapper 2) Second will have the PK and we will expect to use the full number of mappers I'm thinking about test that will ensure that when running all-tables-import, the reset to single mapper will happen only for the affected table and not for other tables that happen to be listed after the affected one. src/docs/user/import.txt <https://reviews.apache.org/r/31839/#comment123229> Nit: Trailing whitespace. Jarcec - Jarek Cecho On March 9, 2015, 1:03 a.m., Venkat Ranganathan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31839/ > ----------------------------------------------------------- > > (Updated March 9, 2015, 1:03 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-trunk > > > Description > ------- > > Provides a new option autoreset-to-one-mapper which instructs sqoop to reset > the number of mappers to one when primary key is not defined for a table and > no split by column is provided (which is the case for import-all-tables). > New tests added and documentation updated > > > Diffs > ----- > > src/docs/user/import-all-tables-purpose.txt d5df88c > src/docs/user/import-all-tables.txt 166825a > src/docs/user/import.txt a6b23df > src/java/org/apache/sqoop/SqoopOptions.java 0070d0b > src/java/org/apache/sqoop/manager/SqlManager.java c81f170 > src/java/org/apache/sqoop/tool/BaseSqoopTool.java b41ee2d > src/java/org/apache/sqoop/tool/ImportTool.java d5bf1eb > src/test/com/cloudera/sqoop/TestAllTables.java 39e8941 > src/test/com/cloudera/sqoop/TestSqoopOptions.java 60460d9 > > Diff: https://reviews.apache.org/r/31839/diff/ > > > Testing > ------- > > All tests pass including newly added tests > > > Thanks, > > Venkat Ranganathan > >
