----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53423/#review156499 -----------------------------------------------------------
Ship it! It seems good. A couple of high level comments: - There is some mix of naming with regards to transaction isolation level. It seems "transaction level" and "isolation level" are used interchangeably in the patch. I'd choose on and go with it for consistency. - Transaction isolation level is only an issue with certain databases (PDW and SQL Server for example). I haven't looked recently, but is this a problem with other databases other than SQL Server esque DBs? You could probably move forward with this patch and think about this later. - Abraham Elmahrek On Nov. 10, 2016, 2:13 p.m., Attila Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53423/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2016, 2:13 p.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 30b4705 > src/java/org/apache/sqoop/manager/SqlManager.java 768507b > src/java/org/apache/sqoop/tool/BaseSqoopTool.java 468bf34 > src/java/org/apache/sqoop/tool/JDBCTransactionLevels.java PRE-CREATION > src/test/org/apache/sqoop/tool/TestImportTool.java PRE-CREATION > > Diff: https://reviews.apache.org/r/53423/diff/ > > > Testing > ------- > > > Thanks, > > Attila Szabo > >
