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

[Backport 2.8] Remove dependency on javax.annotation implementation as OpenSearch now has one. #2804

Closed
wants to merge 1 commit into from

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

Backport 9be79bd from #2801

…w has one. (#2801)

This resolves "JAR hell" issues when installing the plugin.

Signed-off-by: Thomas Farr <[email protected]>
(cherry picked from commit 9be79bd)
@reta
Copy link
Collaborator

reta commented May 30, 2023

FYI, the change in core was not backported to 2.8, only to main and 2.x

@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #2804 (e3e4c06) into 2.8 (ae19524) will increase coverage by 0.07%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                2.8    #2804      +/-   ##
============================================
+ Coverage     61.26%   61.33%   +0.07%     
- Complexity     3316     3321       +5     
============================================
  Files           264      264              
  Lines         18507    18507              
  Branches       3265     3265              
============================================
+ Hits          11338    11351      +13     
+ Misses         5593     5579      -14     
- Partials       1576     1577       +1     

see 5 files with indirect coverage changes

@cwperks
Copy link
Member

cwperks commented May 30, 2023

Closing this PR. The change isn't necessary on the 2.8 branch.

@cwperks cwperks closed this May 30, 2023
@DarshitChanpura
Copy link
Member

@cwperks. We need this change for merging: #2807

@cwperks
Copy link
Member

cwperks commented May 30, 2023

@DarshitChanpura Is there an issue with the Github check? The original PR that introduced the dependency in core is not in core's 2.8 branch: opensearch-project/OpenSearch#7604

@cwperks
Copy link
Member

cwperks commented May 30, 2023

@DarshitChanpura I think the plugin install is failing on the PR you linked to because this artifact (https://artifacts.opensearch.org/snapshots/core/opensearch/2.8.0-SNAPSHOT/opensearch-min-2.8.0-SNAPSHOT-linux-x64-latest.tar.gz) is stale.

That artifact may have been built off of 2.x after this change was merged: opensearch-project/OpenSearch#7779

I don't think that artifact is created from the 2.8 branch yet.

@DarshitChanpura
Copy link
Member

I don't think that artifact is created from the 2.8 branch yet.

Hmm, you are right. Since 2.x hasn't been bumped to 2.9, CI for this branch and 2.x use the same 2.8 artifact. However, I still think we should have this change here, since it applies to all 2.8 artifacts.

@cwperks
Copy link
Member

cwperks commented May 30, 2023

Wouldn't that lead to compile or runtime failures if the jar isn't present?

If the dependency is not coming from core then our build.gradle file needs the direct dependency if the security plugin requires it for compilation or runtime.

@DarshitChanpura
Copy link
Member

Wouldn't that lead to compile or runtime failures if the jar isn't present?

If the dependency is not coming from core then our build.gradle file needs the direct dependency if the security plugin requires it for compilation or runtime.

I see. My understanding was that the change in core was backported to 2.8, but it isn't. So yes we can close this PR.

@peternied peternied deleted the backport/backport-2801-to-2.8 branch July 10, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants