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

Fix ignore unmapped minimum version #1239

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

ryanbogan
Copy link
Member

@ryanbogan ryanbogan commented Oct 11, 2023

Description

Fixes incorrect minimum required version to use ignore_unmapped functionality

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.

@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #1239 (a11644f) into 2.x (def209e) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                2.x    #1239      +/-   ##
============================================
+ Coverage     85.06%   85.12%   +0.06%     
  Complexity     1187     1187              
============================================
  Files           155      155              
  Lines          4881     4881              
  Branches        450      450              
============================================
+ Hits           4152     4155       +3     
+ Misses          530      527       -3     
  Partials        199      199              
Files Coverage Δ
.../main/java/org/opensearch/knn/index/IndexUtil.java 61.97% <100.00%> (ø)

... and 1 file with indirect coverage changes

@heemin32
Copy link
Collaborator

Shouldn't we push the change to main as well?

@ryanbogan
Copy link
Member Author

Shouldn't we push the change to main as well?

Raising a PR for that shortly

@heemin32 heemin32 merged commit 9d2086c into opensearch-project:2.x Oct 11, 2023
85 of 88 checks passed
@ryanbogan ryanbogan deleted the fix_ignore_unmapped branch October 11, 2023 18:50
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 11, 2023
Signed-off-by: Ryan Bogan <[email protected]>
(cherry picked from commit 9d2086c)
@martin-gaievski
Copy link
Member

Shouldn't we push the change to main as well?

+1, we may remove that later from main, but for now it's better to have it in main. FYI, rolling upgrades are supported for all minor versions of current major, plus the last one for the previous major. E.g. if we do 2.12 and then 3.0, then we don't need support for 2.11 in 3.x codeline.

@ryanbogan
Copy link
Member Author

@martin-gaievski @heemin32 looks like main already has the version as 2.11
https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/index/IndexUtil.java#L37C1-L37C1

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