-
Notifications
You must be signed in to change notification settings - Fork 90
Add spec for neural search plugin stats API #850
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the neural plugin is enabled by default place tests under default
. Adding something to tests/plugins/xyz doesn't automatically execute them, that requires updating CI test.yml.
Thanks for taking a look. This API is available from 3.0 beta onwards so I added a new entry in the test spect for it based on the release tag on docker hub following this part of the testing readme. I'm not super familiar with the CI and how the release process works, let me know if there's any additional changes needed |
This CI here may be confusing, so appreciate you hanging here with me. The documentation could use help to make it clearer (wink, wink).
Does this make sense? |
Yup, thanks for the swift reply!
|
Someone should upgrade to beta ASAP to test things before they get released. Including this plugin.
If the plugin is not installed by default then yes. You will need to customize Dockerfile accordingly. I did click the button to run CI on this PR as is, it might yield other problems.
Yep. |
Changes AnalysisCommit SHA: f1b217f API ChangesSummary
ReportThe full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/14889631690/artifacts/3085602527 API Coverage
|
@dblock 3.0.0-beta1 released a few days ago, I've included changes for the version matrix bump in this PR. Happy to break it up into a separate PR as well if that makes more sense. I've also verified neural-search is included in the release artifact by default with the following:
Some question about this:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This repo tests multiple versions, undo all the changes from 2.19.1 to 3.0.0-beta1. You should do the upgrade from 3.0-alpha-1 to 3.0-beta-1 in a separate PR, then add tests for the neural stats here.
Signed-off-by: Andy Qin <[email protected]>
Signed-off-by: Andy Qin <[email protected]>
Signed-off-by: Andy Qin <[email protected]>
Signed-off-by: Andy Qin <[email protected]>
@dblock Addressed remaining comments and also the tests to default since neural search is installed by default. I've also added Tests pass locally with |
Signed-off-by: Daniel (dB.) Doubrovkine <[email protected]>
Changes look good. Fix spec linting / validation and it's good to go. |
Spec Test Coverage Analysis
|
Signed-off-by: Andy Qin <[email protected]>
Signed-off-by: Andy Qin <[email protected]>
Thanks, fixed the failing workflows. All of the following commands run successfully:
|
Description
Spec for neural stats API added to feature branch in this PR: opensearch-project/neural-search#1256
Example API responses and test run output:
Issues Resolved
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.