-
Notifications
You must be signed in to change notification settings - Fork 283
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
Add step to publishPluginZipPublicationToMavenLocal in the build script #3959
Conversation
Signed-off-by: Craig Perkins <[email protected]>
Moved this to draft.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3959 +/- ##
=======================================
Coverage 65.49% 65.50%
=======================================
Files 298 298
Lines 21243 21246 +3
Branches 3460 3461 +1
=======================================
+ Hits 13914 13918 +4
+ Misses 5611 5608 -3
- Partials 1718 1720 +2
|
@cwperks you can refer opensearch-project/flow-framework#226 to fix the above error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cwperks @peternied we shouldn't publish JARs to Apache Maven, but the ZIP distribution only (which is done on the next line)
@reta any idea why flow-framework cannot get the security zip if this line is in security's build.sh?
Shouldn't the artifact be being published for other plugins to consume? This command works for me locally and publishes to |
@reta Totally agreed; however, this change doesn't publish externally - I'm pretty sure you had the same reaction I did when I first saw this PR (I requested changes and then accepted). The build script is used when the distribution build is created and the command is OpenSearch publishes to maven local in its scripts/build.sh |
Why should build script be published? It is owned by |
Signed-off-by: Craig Perkins <[email protected]>
I switched this to using I'm trying to figure out how job-scheduler publishes the zip during distribution build time for dependent plugins. |
@reta This script is used by the distribution build process docs which operates differently than gradle builds. This problem was discovered when attempting to run the integration tests against a distribution build. I'm not if there is another kind of approach that could address this, do you have other ideas? @zelinh Would you mind reviewing this PR and is there more context you can provide for the questions that @.reta has called out? |
Hm ... I see it needs ZIP and with the ZIP publishing to Mavel Local, we should be all set:
|
@zelinh How does opensearch-build allow plugins to have a zipArchive dependency on job-scheduler? From what I am looking at in job-scheduler's build script it is not publishing the zip to maven local. There are lines in job-scheduler's build script to copy the zip to I'm not sure if this is connected, but I see special handling for job-scheduler in opensearch-build. Is this how plugins are able to have a zipArchive dependency on job-scheduler (Example from AD)? |
Switched back to draft to prevent this from being merged. I'd like to understand how zipArchive dependency works for the integ tests in greater detail. Other plugins do not publish the zip to maven local even when other plugins have a zipArchive dependency on them (example from security-analytics which has a zipArchive dependency on alerting). This may not be necessary for the security plugin |
Let me explain this based on what I understood: The
By creating a custom configuration like Now In the dependencies block, the OpenSearch plugin dependencies are associated with the zipArchive configuration: Example as
This means that these dependencies are associated with the So @cwperks the Example from AD even though it has Same with K-NN even though it has Now coming back to
Since security is not publishing to maven local gradle finds and fails the build as dependency not found with configuration
This used to work for neural-search 2.11.0 release where then it does not have explicit Adding @gaiksaya @vibrantvarun @bbarani @jmazanec15 @owaiskazi19 |
@prudhvigodithi Do you think it makes sense to add a step to publish to maven local for the security zip as part of security's build script? This PR should resolve Flow Framework's issue, but I'd like guidance from EE if this is the best way to fix their issue. |
Description
Adds a step to publishPluginZipPublicationToMavenLocal in the security plugin's build script. This will resolve an issue in the distribution build where flow framework is trying to get a "release" (non-snapshot) artifact of the security plugin at build time. See related issue here. The k-NN plugin runs this step in its build.sh script.
Maintenance
Issues Resolved
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.