default build.sh logic add for Gradle custom java zippublish plugin

[prudhvigodithi]

Signed-off-by: pgodithi [email protected]

Description

This change will add custom zippublish plugin publishMavenzipPublicationToZipstagingRepository logic to default build.sh, this can publish plugin zips to maven repo.

Issues Resolved

#1916

Check List

• Commits are signed per the DCO using --signoff

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.

[codecov-commenter]

Codecov Report

Merging #1999 (e53e657) into main (ec1e4dd) will increase coverage by 0.20%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #1999      +/-   ##
============================================
+ Coverage     94.41%   94.61%   +0.20%     
- Complexity       22       27       +5     
============================================
  Files           187      209      +22     
  Lines          3739     4309     +570     
  Branches         29       29              
============================================
+ Hits           3530     4077     +547     
- Misses          203      226      +23     
  Partials          6        6              

Impacted Files	Coverage Δ
src/sign_workflow/signer.py	90.00% <0.00%> (-7.62%)	⬇️
src/test_workflow/perf_test/perf_test_runner.py	95.00% <0.00%> (-5.00%)	⬇️
src/jenkins/BuildManifest.groovy	82.45% <0.00%> (-4.59%)	⬇️
src/run_sign.py	91.66% <0.00%> (-3.79%)	⬇️
src/manifests/component_manifest.py	92.00% <0.00%> (-3.24%)	⬇️
src/test_workflow/integ_test/integ_test_runner.py	97.50% <0.00%> (-2.50%)	⬇️
...rkflow/integ_test/service_opensearch_dashboards.py	96.55% <0.00%> (-1.67%)	⬇️
src/manifests/input_manifest.py	96.55% <0.00%> (-0.68%)	⬇️
src/manifests/build/build_manifest_1_0.py	88.23% <0.00%> (-0.34%)	⬇️
src/manifests/bundle/bundle_manifest_1_0.py	90.32% <0.00%> (-0.31%)	⬇️
... and 77 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec1e4dd...e53e657. Read the comment docs.

[gaiksaya]

Can this be renamed to publishZipsToMavenRepository or something similar? Much simpler and precise.

[prudhvigodithi]

@gaiksaya
so the reason I have added this way is this var MAVEN_ZIP_PUBLISH_TASK is actually referenced same in the java code, https://github.com/opensearch-project/OpenSearch/pull/2988/files#diff-d56f11b358bf18f6e6c28d2a528aa08d4436d01ce9b3c3c45bab1fa970d9cb43R49 and this is the format publishMavenzipPublicationToZipstagingRepository gradle expects for publishing a custom repo (mavenzip) to custom staging repo (zipstaging).

[prudhvigodithi]

just to add this task is coming from the custom gradle plugin (identifies once the plugin is applied).

[gaiksaya]

I see, Yeah I thought this can be changed in upstream PR as well but looks like we can't since its a gradle requirement.

[dblock]

Once the task is available in OpenSearch, doesn't it "just work" with all plugins? Do we need the if?

[prudhvigodithi]

Hey @dblock this task wont show up until a user explicitly add's as apply plugin: 'opensearch.zippublish', so if there are some components that does not want to publish a zip and has not added the plugin and again relies on this default build scripts this task wont show up and will throw an error as task not available.

[dblock]

Hey @dblock this task wont show up until a user explicitly add's as apply plugin: 'opensearch.zippublish', so if there are some components that does not want to publish a zip and has not added the plugin and again relies on this default build scripts this task wont show up and will throw an error as task not available.

Ok, then this is backwards. It should only be added to the plugins that want to publish ZIPs in their own build.sh in their repos.

[prudhvigodithi]

@dblock one way its true, but repos like security, anomaly-detection, cross-cluster-replication (could be more), does use the default build.sh script and still generates the zip, so if we have publish them, do you suggest to maintain their own buildscripts ? But the added if condition should be exiting gracefully if the task does not exists. Just we might need inform user in the case we remove it from default script.

[prudhvigodithi]

My opinion is to add this task to the default script as this will minimize the code changes of existing repos. Since they are already pointing to the default script, they should continue to do, even if they want to publish the zip.
open for discussion.

[dblock]

@dblock one way its true, but repos like security, anomaly-detection, cross-cluster-replication (could be more), does use the default build.sh script and still generates the zip, so if we have publish them, do you suggest to maintain their own buildscripts ? But the added if condition should be exiting gracefully if the task does not exists. Just we might need inform user in the case we remove it from default script.

Yes, that's the design of this system. Unless the task applies to all plugins, don't include it in the default script.

[dblock]

My opinion is to add this task to the default script as this will minimize the code changes of existing repos. Since they are already pointing to the default script, they should continue to do, even if they want to publish the zip. open for discussion.

Many plugins don't use the default script, so it won't really minimize changes that much. Scripts also vary release-by-release/branch-by-branch. And again, this is not the pattern that has been adopted before, otherwise we'd have a default script with tons of "if task XYZ exists run it" for all kinds of other things.

[dblock]

Now, why don't all plugins implement zip publishing?

[prudhvigodithi]

Many plugins don't use the default script, so it won't really minimize changes that much. Scripts also vary release-by-release/branch-by-branch. And again, this is not the pattern that has been adopted before, otherwise we'd have a default script with tons of "if task XYZ exists run it" for all kinds of other things.

So here I was referencing to the plugins that use the default buildscript, who has to re add the script in their repo moving forward.

Now, why don't all plugins implement zip publishing?

Yes they can implement, but need to execute the task publishMavenzipPublicationToZipstagingRepository for this to happen, to call this gradle command ./gradlew publishMavenzipPublicationToZipstagingRepository -Dopensearch.version=$VERSION they need to add it to the build flow and this build flow (using default build.sh) resides in this repo.

[dblock]

Now, why don't all plugins implement zip publishing?

Yes they can implement, but need to execute the task publishMavenzipPublicationToZipstagingRepository for this to happen, to call this gradle command ./gradlew publishMavenzipPublicationToZipstagingRepository -Dopensearch.version=$VERSION they need to add it to the build flow and this build flow (using default build.sh) resides in this repo.

If this were the case (all plugins implement this action), then you can add it to the default script. It would be similar to how all plugins implement build.

[prudhvigodithi]

If this were the case (all plugins implement this action), then you can add it to the default script. It would be similar to how all plugins implement build.

Make sense @dblock , also I think it would be not a good experience for repo users, to ask them to create and maintain a custom script, just because they want the zips to be in maven.

[dblock]

I can't agree to having f ./gradlew tasks -Dopensearch.version=$VERSION --all | grep -qw "^$MAVEN_ZIP_PUBLISH_TASK", this is backwards, and not how we've done customization across repos. Add the task to all repos and add it to the default script without the conditional and customize everything else.

[prudhvigodithi]

I can't agree to having f ./gradlew tasks -Dopensearch.version=$VERSION --all | grep -qw "^$MAVEN_ZIP_PUBLISH_TASK", this is backwards, and not how we've done customization across repos. Add the task to all repos and add it to the default script without the conditional and customize everything else.

Hey @dblock as you know right now, not all plugins, use bundlePlugin task to generate the zip as well, so these repos cant even use the new plugin until they migrate the zip generation using bundlePlugin task, so even before adding/executing the MAVEN_ZIP_PUBLISH_TASK task, these repos has to migrate to this bundlePlugin task, then next they can start using the plugin and then off course the publish task can then be added, while this all happens, if we cant add this task to default build script, then the plugins that depend on the default build.sh has to wait until the other repos migrate and start using bundlePlugin task or create their own build.sh and start using it.

[dblock]

I can't agree to having f ./gradlew tasks -Dopensearch.version=$VERSION --all | grep -qw "^$MAVEN_ZIP_PUBLISH_TASK", this is backwards, and not how we've done customization across repos. Add the task to all repos and add it to the default script without the conditional and customize everything else.

Hey @dblock as you know right now, not all plugins, use bundlePlugin task to generate the zip as well, so these repos cant even use the new plugin until they migrate the zip generation using bundlePlugin task, so even before adding/executing the MAVEN_ZIP_PUBLISH_TASK task, these repos has to migrate to this bundlePlugin task, then next they can start using the plugin and then off course the publish task can then be added, while this all happens, if we cant add this task to default build script, then the plugins that depend on the default build.sh has to wait until the other repos migrate and start using bundlePlugin task or create their own build.sh and start using it.

This is a hack that we will never undo, so I am opposed to it. Implement the new task for 1-2 repos where it's easy (e.g. job-scheduler), then run a campaign where each repo does the work themselves of migrating to bundlePlugin and using this new task, then add it to the default script.

[prudhvigodithi]

Hey, just coming back to this PR again, since the main gradle plugin PR was merged, I have started creating campaign for plugins components to start using this gradle plugin, some of the issues that can come up are:

• Generation of zips by not using budlePlugin task.
• Even though budlePlugin task exists, not having a managed build script (relying of default build script).
• Creating/Adding managed build script for their own repo as default build script does not yet have publishPluginZipPublicationToZipStagingRepository task added

Will this block the plugins to publish the zips to maven that are already doing right by using budlePlugin task but still does not own the managed build script and rely on default build.sh?

@dblock

[dblock]

@prudhvigodithi I think you should do this work for 1 plugin (e.g. job-scheduler) before creating a campaign. And document exactly what needs to be done: so things like use bundlePlugin should be in there, e.g. "1. Rewrite build to use budlePlugin like so [link] if you aren't doing it already."

[prudhvigodithi]

@prudhvigodithi I think you should do this work for 1 plugin (e.g. job-scheduler) before creating a campaign. And document exactly what needs to be done: so things like use bundlePlugin should be in there, e.g. "1. Rewrite build to use budlePlugin like so [link] if you aren't doing it already."

Sure @dblock, However I have already created an enhancement issue across all plugin repos, regrading this :)
I will start with job-scheduler (issue)

[dblock]

@prudhvigodithi I think you should do this work for 1 plugin (e.g. job-scheduler) before creating a campaign. And document exactly what needs to be done: so things like use bundlePlugin should be in there, e.g. "1. Rewrite build to use budlePlugin like so [link] if you aren't doing it already."

Sure @dblock, However I have already created an enhancement issue across all plugin repos, regrading this :) I will start with job-scheduler (issue)

Consider https://github.com/opensearch-project/opensearch-plugin-template-java first!

[prudhvigodithi]

Hey I have removed the if condition as now all the plugins should have the publishPluginZipPublicationToZipStagingRepository gradle task. Should we still consider to add publishPluginZipPublicationToZipStagingRepository to default build.sh?
Thoughts @dblock @gaiksaya @peterzhuamazon @bbarani
Thank you

[dblock]

I would just say "Publish plugin zips to maven"