> On Dec. 2, 2016, 11:09 a.m., Attila Szabo wrote: > > src/java/org/apache/sqoop/SqoopOptions.java, lines 1037-1043 > > <https://reviews.apache.org/r/54206/diff/2/?file=1574599#file1574599line1037> > > > > What about inline into something like this: > > this.throwOnError = (System.getProperty(SQOOP_RETHROW_PROPERTY) != null) > > > > Could you explain why do we prefer the broader format?
I have changed to the inline format, thanks. > On Dec. 2, 2016, 11:09 a.m., Attila Szabo wrote: > > src/java/org/apache/sqoop/tool/BaseSqoopTool.java, lines 289-293 > > <https://reviews.apache.org/r/54206/diff/2/?file=1574600#file1574600line289> > > > > Why is it required to handle the RuntimeException differently? > > > > If I'm not mistaken in the old version we'd wrapped all type of > > exceptions, and no exclusion was applied to RuntimeExceptions? > > > > What is your intention here? Thanks for pointing it out, Attila! I think not wrapping an exception into RuntimeException when it is a RuntimeException itself is more straigtforward and clear so I would stay with this implementation instead of the old one. - Boglarka ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54206/#review157727 ----------------------------------------------------------- On Dec. 2, 2016, 10:59 a.m., Boglarka Egyed wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54206/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2016, 10:59 a.m.) > > > Review request for Sqoop and Attila Szabo. > > > Bugs: SQOOP-3053 > https://issues.apache.org/jira/browse/SQOOP-3053 > > > Repository: sqoop-trunk > > > Description > ------- > > A new cmd line argument has been added for throwing a RuntimeException on > error which can be used through SqoopOptions from now. To ensure backward > compatibily the deafult value is set according to the sqoop.throwOnError > system property which was used for this feature before. > > > Diffs > ----- > > src/java/com/cloudera/sqoop/Sqoop.java > c3d9725b010f69be9b3664396d48974fb5f0d370 > src/java/org/apache/sqoop/Sqoop.java > 93736a6ba328648d5d6cbe6d53e917db52d304f9 > src/java/org/apache/sqoop/SqoopOptions.java > ef26f1628995bab5e8472339cc201d10e4093af2 > src/java/org/apache/sqoop/tool/BaseSqoopTool.java > 3ed0f779b047ff46277a1b2fe78b5666e5bff990 > src/java/org/apache/sqoop/tool/CodeGenTool.java > b3107a2b0eb5b794a1d21b8953c3854ed4cf67f4 > src/java/org/apache/sqoop/tool/CreateHiveTableTool.java > 427376d9fa226e33cb1a752e88f69603a1f7ffbd > src/java/org/apache/sqoop/tool/ExportTool.java > 89f859013880cba0639ed63201f1a1234767f39a > src/java/org/apache/sqoop/tool/ImportAllTablesTool.java > 0952c1125d99628591f956d160fb3a369e78c681 > src/java/org/apache/sqoop/tool/ImportTool.java > d3f8b935de95a541cb29240795502a4c663f2105 > src/java/org/apache/sqoop/tool/MergeTool.java > 09589d81c6529428031bb1bc56c3c5f9ae0906af > src/test/com/cloudera/sqoop/TestSqoopOptions.java > f9d1d54bd06531aae955d5dccc22e1d799cb1fb8 > src/test/org/apache/sqoop/tool/TestBaseSqoopTool.java PRE-CREATION > > Diff: https://reviews.apache.org/r/54206/diff/ > > > Testing > ------- > > New unit test cases have been added. > > > Thanks, > > Boglarka Egyed > >