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 1.2] Allow null value for params in method mappings #357

Merged

Conversation

jmazanec15
Copy link
Member

Allow user to input null value for parameters field for KNNMethodContext
and MethodComponentContext. Adds tests to reproduce Mapping Parsing Error when the parameters take
the value null as well as BWC tests.

Backport: #354

Signed-off-by: John Mazanec [email protected]
(cherry picked from commit b08127c)

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.

Allow user to input null value for parameters field for KNNMethodContext
and MethodComponentContext. Adds tests to reproduce Mapping Parsing Error when the parameters take
the value null as well as BWC tests.

Signed-off-by: John Mazanec <[email protected]>
(cherry picked from commit b08127c)
@jmazanec15
Copy link
Member Author

BWC tests were not introduced until after 1.2 so they could not be backported back.

@codecov-commenter
Copy link

Codecov Report

Merging #357 (3917fd6) into 1.2 (af3c80a) will increase coverage by 0.11%.
The diff coverage is 78.57%.

@@             Coverage Diff              @@
##                1.2     #357      +/-   ##
============================================
+ Coverage     83.19%   83.31%   +0.11%     
- Complexity      864      875      +11     
============================================
  Files           123      123              
  Lines          3779     3799      +20     
  Branches        358      363       +5     
============================================
+ Hits           3144     3165      +21     
+ Misses          475      474       -1     
  Partials        160      160              
Impacted Files Coverage Δ
...g/opensearch/knn/index/MethodComponentContext.java 91.76% <75.00%> (+1.09%) ⬆️
...ava/org/opensearch/knn/index/KNNMethodContext.java 93.25% <100.00%> (+0.23%) ⬆️
...n/java/org/opensearch/knn/index/KNNIndexShard.java 93.75% <0.00%> (+0.72%) ⬆️
...a/org/opensearch/knn/index/codec/KNNCodecUtil.java 85.71% <0.00%> (+1.50%) ⬆️
...c/main/java/org/opensearch/knn/index/KNNQuery.java 77.77% <0.00%> (+5.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af3c80a...3917fd6. Read the comment docs.

Copy link
Member

@vamshin vamshin left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@jmazanec15 jmazanec15 merged commit 319ad91 into opensearch-project:1.2 Apr 5, 2022
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.

4 participants