> On June 19, 2018, 7:16 a.m., Szabolcs Vasas wrote: > > src/docs/user/connectors.txt > > Line 151 (original), 151 (patched) > > <https://reviews.apache.org/r/67524/diff/4/?file=2041992#file2041992line151> > > > > Shouldn't we just say "This will retry"?
Certainly sounds better. > On June 19, 2018, 7:16 a.m., Szabolcs Vasas wrote: > > src/java/org/apache/sqoop/manager/SQLServerManager.java > > Line 54 (original), 48 (patched) > > <https://reviews.apache.org/r/67524/diff/4/?file=2041994#file2041994line54> > > > > There is an extra space here, please remove it. Done > On June 19, 2018, 7:16 a.m., Szabolcs Vasas wrote: > > src/java/org/apache/sqoop/manager/SQLServerManager.java > > Line 156 (original) > > <https://reviews.apache.org/r/67524/diff/4/?file=2041994#file2041994line164> > > > > This input format does not seem to be set by > > formatConfigurator.configureContextForImport(context, splitColumn) even if > > the splitColumn is null. Good catch! This was done because I've deduped parts of the importTable and the importQuery functions and only the former executed the statement you refer to. > On June 19, 2018, 7:16 a.m., Szabolcs Vasas wrote: > > src/java/org/apache/sqoop/manager/SQLServerManager.java > > Line 206 (original) > > <https://reviews.apache.org/r/67524/diff/4/?file=2041994#file2041994line214> > > > > Just to double check: in the new implementation you call > > configureConnectionRecoveryForExport instead of > > configureConnectionRecoveryForUpdate but that should be fine since in the > > original version configureConnectionRecoveryForExport and > > configureConnectionRecoveryForUpdate were duplicates, so you dropped > > configureConnectionRecoveryForUpdate and kept > > configureConnectionRecoveryForExport, right? yes, those two were duplicates - Fero ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67524/#review204978 ----------------------------------------------------------- On June 19, 2018, 10:28 a.m., Fero Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67524/ > ----------------------------------------------------------- > > (Updated June 19, 2018, 10:28 a.m.) > > > Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas. > > > Bugs: SQOOP-3333 > https://issues.apache.org/jira/browse/SQOOP-3333 > > > Repository: sqoop-trunk > > > Description > ------- > > This change is about changing the default behavior of the MS SQL connector > from resilient to non-resilient. I was aiming for the fewest possible > modifications while also removed double negation where previously present. > > I've refactored the context configuration into a separate class. > > I've also changed the documentation of the non-resilient flag and added a > note about the implicit requirement of the feature (that the split-by column > has to be unique and ordered in ascending order). > > I plan to expand the documentation more in SQOOP-3332, as the (now named) > resilient flag works not just for export, but import as well (queries and > tables). > > I've also added new tests that cover what classes get loaded in connection > with the resilient option. Also, I've refactored SQL Server import tests and > added a few more cases for better coverage. (The query import uses a > different method and wasn't covered by these tests at all.) > > > Diffs > ----- > > src/docs/user/connectors.txt 7c540718 > src/java/org/apache/sqoop/manager/ExportJobContext.java 773cf742 > src/java/org/apache/sqoop/manager/SQLServerManager.java b136087f > src/java/org/apache/sqoop/manager/SqlServerManagerContextConfigurator.java > PRE-CREATION > src/test/org/apache/sqoop/manager/sqlserver/SQLServerManagerImportTest.java > c83c2c93 > > src/test/org/apache/sqoop/manager/sqlserver/TestSqlServerManagerContextConfigurator.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/67524/diff/5/ > > > Testing > ------- > > Added new unit tests for SqlServerConfigurator. > unit and 3rd party tests. > ant docs ran succesfully. > manual testing. > > > Thanks, > > Fero Szabo > >
