----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40350/#review106679 -----------------------------------------------------------
Thanks for putting the patch together Dian. I do have few high level notes: 1) It seems that the shade plugin is configured in way that it adds classes from all dependencies to the connector jar. E.g. when I un-jar the connector, I get following content: creating: org/apache/log4j/spi/ inflating: org/apache/log4j/spi/AppenderAttachable.class inflating: org/apache/log4j/spi/Configurator.class inflating: org/apache/log4j/spi/DefaultRepositorySelector.class I believe that our decision was to put all dependency jars to lib/ folder inside the connector jar, right? Putting all the dependencies directly to the connector jar is a bit concerning because that means that all those dependencies will be available on Sqoop Server classpath which in turn can cause unexpected behavior. E.g. in this way the connector's can have isolated classpath, but Sqoop 2 server itself will not. 2) I see bunch of dependencies that I believe shouldn't be there - org/apache/sqoop/* or classes from joda-time are good examples. We've discussed the problem here [1] and I believe that the decision was that those dependencies will be used from the "parent" classloader (=Sqoop classpath) rather then from connector classpath, right? Jarcec Links: 1: https://issues.apache.org/jira/browse/SQOOP-2634 - Jarek Cecho On Nov. 16, 2015, 2:44 p.m., Dian Fu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40350/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2015, 2:44 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2691 > https://issues.apache.org/jira/browse/SQOOP-2691 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Per the design of SQOOP-2634, we need firstly use maven shade plugin to > package connector's dependencies with the connector jar itself. > > > Diffs > ----- > > connector/connector-ftp/pom.xml 41ea026 > connector/connector-generic-jdbc/pom.xml 052f06d > connector/connector-hdfs/pom.xml 022a024 > connector/connector-kafka/pom.xml e0f0684 > > connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaToInitializer.java > 923d1aa > connector/connector-kite/pom.xml d8eaa8e > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteFromInitializer.java > 28c5bac > > connector/connector-kite/src/main/java/org/apache/sqoop/connector/kite/KiteToInitializer.java > 50daba0 > connector/connector-oracle-jdbc/pom.xml 8186b3a > connector/connector-sftp/pom.xml 312ac61 > > connector/connector-sftp/src/main/java/org/apache/sqoop/connector/sftp/SftpToInitializer.java > bfb51ac > pom.xml f33958c > > Diff: https://reviews.apache.org/r/40350/diff/ > > > Testing > ------- > > > Thanks, > > Dian Fu > >
