----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7143/#review11650 -----------------------------------------------------------
Hi David, thank you very much for identifying this discrepancy and providing patch. I do have just one comment: src/java/org/apache/sqoop/hive/HiveImport.java <https://reviews.apache.org/r/7143/#comment25169> As the com.cloudera.sqoop.util.ExitSecurityException inherits from org.apache.sqoop.util.ExitSecurityException, I believe that this change might not be necessary as the com.cloudera.* exception should be caught later in the code even if the catch statement is using org.apache.sqoop.util.ExitSecurityException, right? Jarcec - Jarek Cecho On Sept. 18, 2012, 6:30 a.m., David Robson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7143/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2012, 6:30 a.m.) > > > Review request for Sqoop. > > > Description > ------- > > Throw the exception from the old namespace to support anyone checking the old > exception. Also update HiveImport to check the new namespace. > > > This addresses bug SQOOP-607. > https://issues.apache.org/jira/browse/SQOOP-607 > > > Diffs > ----- > > src/java/org/apache/sqoop/hive/HiveImport.java ce34286 > src/java/org/apache/sqoop/util/SubprocessSecurityManager.java 4951627 > > Diff: https://reviews.apache.org/r/7143/diff/ > > > Testing > ------- > > Created a mock Hive CLI that calls System.exit(0). Can see it now works. > Was trying to create an automated test but the problem was once I created the > mock class it was used for the other tests as well instead of the shell > script. > The easiest way I can see to create a unit test would be to allow the Hive > CLI class to be specified as a parameter - that way it could be overridden in > a unit test to a mock class. > Was also thinking about loading the class at runtime but this seemed like a > bit of work for a small bug fix. > > > Thanks, > > David Robson > >
