-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-4861] [build] Package optional project artifacts #2664
[FLINK-4861] [build] Package optional project artifacts #2664
Conversation
I think this is a good idea. Would it make sense to have a sub directory for each connector, with its specific dependencies? I am also wondering if there are maybe some unwanted test dependencies in the binary build. For example |
@StephanEwen thanks for looking at this. In this first cut I included all the build artifacts. Should connectors be included? We should also consider allowing jars to be placed in subdirectories of Flink's |
Connectors are already included, correct? |
What do you mean by "We should also consider allowing jars to be placed in subdirectories of Flink's lib folder."? |
The mailing list conversation only included an indirect reference to including connectors in this I'm not seeing Flink load jar files from a subdirectory of |
Subdirectories of If we package any optional dependencies, I think the connectors would be very worthwhile. |
4fe6870
to
d61be19
Compare
@StephanEwen latest commit assembles connectors into separate directories. I'll create a ticket for loading jar files from subdirectories of |
I need to rework this since additional classes are being pulled into the uber jar. |
Package the Flink connectors, metrics, and libraries into subdirectories of a new opt directory in the release/snapshot tarballs.
This new module assembles optional Flink packages with dependencies which are then copied by flink-dist into build-target. A separate module is required to prevent these packages from inclusion in the uber jar (or otherwise requiring a tremendious, brittle exclusion list).
d61be19
to
d2170ba
Compare
@StephanEwen the latest commit uses a separate module to prevent the optional packages and dependencies from being included in the |
I tried out the change and I like the idea. |
@rmetzger it appears that project artifacts are not included as transitive dependencies and I had overlooked 0.9 as a dependency for the 0.10 connector. After correcting this the provided hierarchy is as follows:
|
Thank you for fixing the issue so quickly. What do you think? |
@rmetzger, I have not been able to improve on the current configuration. This implementation is only including so there shouldn't be the same risk of failing to exclude an unneeded dependency. How would creating separate assembly descriptors be beneficial? |
a1cf913
to
428f01d
Compare
I thought that transitive dependencies are resolved in the scope of assembly descriptors. But I'm not so sure about that anymore. |
How about just building a fat jar for each connector / library? |
I had the same thought. We could add the maven assembly plugin / shade plugin to each connector / library to build a fat jar, and then add some logic to flink-dist to collect these fat jars into the final dist. |
@StephanEwen @rmetzger, why would a user copy an optional fat jar rather than having it included in their uber jar? By creating fat jars, do we not have the potential for duplicate dependencies if more than one fat jar is included on the classpath? I don't think we can shade since the user may be depending on the transitive dependencies. |
@StephanEwen @rmetzger, as I revisit this, I'm still questioning the viability of shading an uber jar. For example, a user depending on a Kafka connector. Normally the dependency would be packaged in the user's uber jar. Instead, the user could mark the dependency as provided and copy the connector jar into |
@greghogan I think that when an immediate dependency is "provided", then its transitive dependencies are not pulled. If a user adds a "provided" Consequently, all transitive dependencies should be added to the |
New implementation using maven-shade-plugin in #3000. |
@greghogan I guess this PR is invalid by now? |
Package the Flink connectors, metrics, and libraries into subdirectories of a new opt directory in the release/snapshot tarballs.
The resultant directory with Flink jars and transitive dependencies: