> On Sept. 18, 2012, 6:49 a.m., Jarek Cecho wrote: > > src/java/org/apache/sqoop/hive/HiveImport.java, lines 42-48 > > <https://reviews.apache.org/r/7143/diff/1/?file=156128#file156128line42> > > > > 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? > > David Robson wrote: > Yes - with the other change this is not necessary - I included it as I > thought removing a reference to the old namespace (where possible) is > probably good. I can remove this bit and re-submit if you like?
It would be great if you could remove this fragment and resubmit your patch. Please upload your new patch both to review board and JIRA (as a file attachment). Jarcec - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7143/#review11650 ----------------------------------------------------------- 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 > >
