----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53423/#review154901 -----------------------------------------------------------
Ship it! Hi Attila, The fix looks good to me however I have an idea which may be an overkill to add now but I think it could make the code a bit simpler. We could introduce an enum for the isolation levels, which would encapsulate the name and the int code of the isolation. Using this enum we could eliminate the switch statement from the BaseSqoopTool and the unit test could be also simpler. What do you think? - Szabolcs Vasas On Nov. 4, 2016, 3:52 a.m., Attila Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53423/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2016, 3:52 a.m.) > > > Review request for Sqoop, Abraham Elmahrek, Abraham Fine, Boglarka Egyed, > Anna Szonyi, Szabolcs Vasas, and Erzsebet Szilagyi. > > > Bugs: SQOOP-2349 > https://issues.apache.org/jira/browse/SQOOP-2349 > > > Repository: sqoop-trunk > > > Description > ------- > > I've introduced a cmd line parameter for being able to set the metadata > transaction levels (defined on java.sql.Connection) manually if necessary. > The change is backward compatible, so by default SQOOP gonna still use > READ_COMMITTED. > > > Diffs > ----- > > src/java/org/apache/sqoop/SqoopOptions.java e14a0b7 > src/java/org/apache/sqoop/manager/SqlManager.java 768507b > src/java/org/apache/sqoop/tool/BaseSqoopTool.java 13a9697 > src/test/org/apache/sqoop/tool/TestImportTool.java PRE-CREATION > > Diff: https://reviews.apache.org/r/53423/diff/ > > > Testing > ------- > > > Thanks, > > Attila Szabo > >
