> On Nov. 16, 2015, 4:28 p.m., Jarek Cecho wrote: > > 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
Hi Jarcec, Thanks for your review comments. Make sense to me. I misunderstood that we agree to use maven shade plugin to package the dependencies. But I realize that put the dependencies into a lib/ folder is the correct way and have finished part of the code. Will update the patch when the new patch is ready. - Dian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40350/#review106679 ----------------------------------------------------------- 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 > >
