Adding gradle publish task to ppl & protocol module to create slim jars #2445
Conversation
Signed-off-by: zane-neo <[email protected]>
Signed-off-by: zane-neo <[email protected]>
Signed-off-by: zane-neo <[email protected]>
zane-neo
requested review from
pjfitzgibbons,
ps48,
kavithacm,
derek-ho,
joshuali925,
dai-chen,
YANG-DB,
rupal-bq,
mengweieric,
vmmusings,
Swiddis,
penghuo,
seankao-az,
MaxKsyunz,
Yury-Fridlyand,
anirudha,
forestmvey,
acarbonetto and
GumpacG
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.x #2445 +/- ##
=========================================
Coverage 95.41% 95.41%
Complexity 5022 5022
=========================================
Files 479 479
Lines 14072 14072
Branches 933 933
=========================================
Hits 13427 13427
Misses 625 625
Partials 20 20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| groupId = 'org.opensearch' | ||
| artifactId = 'opensearch-protocol' |
There was a problem hiding this comment.
The whole SQL plugin artifact is published here: https://mvnrepository.com/artifact/org.opensearch.plugin/opensearch-sql-plugin
So we want to publish its slim jar to a different group and artifact ID?
There was a problem hiding this comment.
This won't change the plugin's location in maven repo, this only controls the slim jar's location and it seems opensearch jars are always under org.opensearch folder: https://mvnrepository.com/search?q=org.opensearch&p=1
There was a problem hiding this comment.
PPL transport model class is in plugin module. So how we add this dependency? Just thinking if we have to depend on the current SQL artifact https://mvnrepository.com/artifact/org.opensearch.plugin/opensearch-sql-plugin anyway, probably we don't need this slim jar?
We still need these slim jars since when building ml-common plugin(which is a shadow jar), it'll recursively find all dependent jars to its classpath, without these jars, it's not possible to build a ml-common plugin successfully. Also, in runtime, different plugins are loaded by different classloaders, so ml-commons can not see the classes under sql plugin, without these jars under ml-commons plugin, there'll be runtime errors. |
I downloaded the current SQL zip artifact published to Maven to double check. It actually contains all jar files, including plugin, PPL and protocol module jar. I was thinking any chance your gradle depends on it directly and remove those you feel not needed at runtime? Let me know if I missed anything. |
We do have another option which is to create a fat/shadow jar of plugin, we tried this option and have a POC PR here: Hailong-am@b7635ca. But this could have impact on publishing plugin zip. Publishing a slim jar is always better than shadow jar which can avoid a lot of jar conflict, so we prefer to publish slim jars like in this PR. |
|
Using zip extraction approach to create a fat jar instead of publish slim jars. |
Description
Adding gradle publish task to ppl & protocol module to create slim jars
Issues Resolved
Adding gradle publish task to ppl & protocol module to create slim jars
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.