Skip to content
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

Update build script to publish to maven local #596

Merged
merged 3 commits into from
Oct 27, 2022

Conversation

jmazanec15
Copy link
Member

Description

Updates build script to publish zip to local maven. This allows plugins like neural search to pick up the zip as a dependency.

Fixes bug in build script where group was being set to org.opensearch as opposed to org.opensearch.plugin.

Publish to maven passed:

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.5/userguide/command_line_interface.html#sec:command_line_warnings

Execution optimizations have been disabled for 1 invalid unit(s) of work during this build to ensure correctness.
Please consult deprecation warnings for more details.

BUILD SUCCESSFUL in 16s
38 actionable tasks: 38 executed

Tried with variety
./gradlew :publishToMavenLocal -Dbuild.snapshot=true -Dbuild.version_qualifier=rc1
./gradlew :publishToMavenLocal -Dbuild.snapshot=true
./gradlew :publishToMavenLocal -Dbuild.snapshot=false

ls ~/.m2/repository/org/opensearch/plugin/opensearch-knn/
3.0.0.0				3.0.0.0-SNAPSHOT		3.0.0.0-rc1-SNAPSHOT		maven-metadata-local.xml

Issues Resolved

opensearch-project/neural-search#31

Check List

  • Commits are signed as 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.

Updates build script to publish zip to local maven. This allows plugins
like neural search to pick up the zip as a dependency.

Fixes bug in build script where group was being set to org.opensearch as
opposed to org.opensearch.plugin.

Signed-off-by: John Mazanec <jmazane@amazon.com>
@jmazanec15 jmazanec15 requested a review from a team October 26, 2022 20:57
@jmazanec15 jmazanec15 added backport 2.x Infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc. labels Oct 26, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #596 (2d0c916) into main (5bb7a3f) will decrease coverage by 0.25%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #596      +/-   ##
============================================
- Coverage     84.77%   84.51%   -0.26%     
+ Complexity     1059     1054       -5     
============================================
  Files           149      149              
  Lines          4301     4301              
  Branches        382      382              
============================================
- Hits           3646     3635      -11     
- Misses          480      489       +9     
- Partials        175      177       +2     
Impacted Files Coverage Δ
...va/org/opensearch/knn/index/KNNCircuitBreaker.java 60.00% <0.00%> (-20.00%) ⬇️
...ain/java/org/opensearch/knn/index/KNNSettings.java 80.88% <0.00%> (-2.21%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

build.gradle Outdated
@@ -53,7 +53,7 @@ ext {

allprojects {

group = 'org.opensearch'
group = 'org.opensearch.plugin'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not ideal because we set group=org.opensearch so jars can be pushed to org/opensearch/<>.
We then set groupId=org.opensearch.plugin so zips can be pushed to org/opensearch/plugin/<>.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For k-NN, we dont puish any jars at the moment.

Copy link
Contributor

@reta reta Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmazanec15 you could keep group=org.opensearch there and use groupId=org.opensearch.plugin for ZIPs, it will fix the issue:

   > Invalid publication 'nebula': supplied groupId (org.opensearch) does not match value from POM file (org.opensearch.plugin). Cannot edit groupId directly in the POM file.

Copy link
Member Author

@jmazanec15 jmazanec15 Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reta I believe this is what we do now:

  1. https://github.com/opensearch-project/k-NN/blob/main/build.gradle#L56
  2. https://github.com/opensearch-project/k-NN/blob/main/build.gradle#L91

I think I can leave as is, but change build script to use:

./gradlew publishPluginZipPublicationToMavenLocal

Signed-off-by: John Mazanec <jmazane@amazon.com>
@jmazanec15 jmazanec15 requested review from reta, naveentatikonda and martin-gaievski and removed request for reta October 27, 2022 00:00
Signed-off-by: John Mazanec <jmazane@amazon.com>
@jmazanec15 jmazanec15 merged commit 5323699 into opensearch-project:main Oct 27, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 27, 2022
Updates build script to publish zip to local maven. This allows plugins
like neural search to pick up the zip as a dependency.

Signed-off-by: John Mazanec <jmazane@amazon.com>
(cherry picked from commit 5323699)
@heemin32 heemin32 added the v2.4.0 'Issues and PRs related to version v2.4.0' label Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Infrastructure Changes to infrastructure, testing, CI/CD, pipelines, etc. v2.4.0 'Issues and PRs related to version v2.4.0'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants