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

Change imports to align with core lucene upgrade #1147

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

ryanbogan
Copy link
Member

Description

OpenSearch core recently updated the lucene version, rendering some constants used in k-NN obsolete. This PR updates to the new versions of those constants.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • 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.

@ryanbogan
Copy link
Member Author

This has not been backported to 2.x on core, so there is no need for us to backport either.

@ryanbogan
Copy link
Member Author

Can we skip the CHANGELOG on this?

@navneet1v
Copy link
Collaborator

Can we skip the CHANGELOG on this?

yes I just added skip-changelog label

@heemin32
Copy link
Collaborator

Can we skip the CHANGELOG on this?

Shouldn't we put it as Maintenance?

@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Merging #1147 (6a82705) into main (ed7171e) will decrease coverage by 0.07%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #1147      +/-   ##
============================================
- Coverage     85.01%   84.95%   -0.07%     
  Complexity     1184     1184              
============================================
  Files           159      159              
  Lines          4813     4813              
  Branches        433      433              
============================================
- Hits           4092     4089       -3     
- Misses          525      528       +3     
  Partials        196      196              
Files Changed Coverage Δ
...opensearch/knn/index/mapper/LuceneFieldMapper.java 71.15% <ø> (ø)
.../java/org/opensearch/knn/index/util/KNNEngine.java 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@martin-gaievski
Copy link
Member

did you check why bwc for rolling upgrade are failing? Also I wonder if we need to update our codec with this lucene version bump, I think current version of lucene codec is 9.5, and core has switched to 9.8

@ryanbogan
Copy link
Member Author

ryanbogan commented Sep 20, 2023

did you check why bwc for rolling upgrade are failing? Also I wonder if we need to update our codec with this lucene version bump, I think current version of lucene codec is 9.5, and core has switched to 9.8

It's failing with a connect exception, but that's not unique to this PR. I'm not sure what's causing that exception, but it's present on every PR I've raised recently.

As for the codec version, I'm not familiar with how frequently we update them. The core upgrade was from one snapshot of 9.8 to a newer snapshot of 9.8.

@navneet1v navneet1v merged commit ca5e483 into opensearch-project:main Sep 25, 2023
36 of 51 checks passed
@navneet1v
Copy link
Collaborator

did you check why bwc for rolling upgrade are failing? Also I wonder if we need to update our codec with this lucene version bump, I think current version of lucene codec is 9.5, and core has switched to 9.8

@martin-gaievski we should move to latest codec. @heemin32 can we migrate to codec as per latest update of core, as you are the release owner for 2.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants