You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Dian Fu <di...@gmail.com> on 2015/11/16 15:44:28 UTC
Review Request 40350: Sqoop2: Use maven shade plugin to package
connector dependencies with the connector jar itself
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40350/
-----------------------------------------------------------
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
Re: Review Request 40350: Sqoop2: Use maven shade plugin to package
connector dependencies with the connector jar itself
Posted by Dian Fu <di...@gmail.com>.
> 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
>
>
Re: Review Request 40350: Sqoop2: Use maven shade plugin to package
connector dependencies with the connector jar itself
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
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
>
>