----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10229/#review18705 -----------------------------------------------------------
Thanks for the changes. They look good. There are a few nits and couple of questions below. Thanks src/java/org/apache/sqoop/util/ClassLoaderStack.java <https://reviews.apache.org/r/10229/#comment39132> Why is the JarURL format getting changed. File(jarFile).toURI().toURL() will still have the scheme as file right? src/test/com/cloudera/sqoop/hive/TestHiveImport.java <https://reviews.apache.org/r/10229/#comment39131> Nit: Trailing spaces src/test/com/cloudera/sqoop/io/TestNamedFifo.java <https://reviews.apache.org/r/10229/#comment39133> Good point on NamedFifo. Nit. Can you add log message saying that it is not supported src/test/com/cloudera/sqoop/lib/TestClobRef.java <https://reviews.apache.org/r/10229/#comment39135> I am not sure the recursive delete can affect other test behavior. I suggest not change additional generic behavior as part of this patch and address them in separate JIRAs Another question. String tmpDir = System.getProperty("test.build.data", "/tmp/"); Also, did you want to change the above for windows as /tmp may not be always available in the current drive). I was wondering something like getting the environment variable TMPDIR and use it instead of /tmp. src/test/com/cloudera/sqoop/orm/TestClassWriter.java <https://reviews.apache.org/r/10229/#comment39134> Nit - tabs instead of spaces - Venkat Ranganathan On April 1, 2013, 8:48 p.m., Ahmed El Baz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10229/ > ----------------------------------------------------------- > > (Updated April 1, 2013, 8:48 p.m.) > > > Review request for Sqoop. > > > Description > ------- > > Please see SQOOP-955 for details > > > This addresses bug SQOOP-955. > https://issues.apache.org/jira/browse/SQOOP-955 > > > Diffs > ----- > > src/java/org/apache/sqoop/hive/HiveImport.java ce34286 > src/java/org/apache/sqoop/orm/CompilationManager.java 92effb5 > src/java/org/apache/sqoop/util/ClassLoaderStack.java 8e41cb3 > src/test/com/cloudera/sqoop/hive/TestHiveImport.java 170bc66 > src/test/com/cloudera/sqoop/io/TestNamedFifo.java 9f0a585 > src/test/com/cloudera/sqoop/lib/TestBlobRef.java 2054a9b > src/test/com/cloudera/sqoop/lib/TestClobRef.java 8cb05e9 > src/test/com/cloudera/sqoop/orm/TestClassWriter.java 3b77571 > testdata/hive/bin/hive.cmd PRE-CREATION > > Diff: https://reviews.apache.org/r/10229/diff/ > > > Testing > ------- > > > Thanks, > > Ahmed El Baz > >
