-
Notifications
You must be signed in to change notification settings - Fork 114
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
GET SM policies return empty list when ism config index does not exist #1072
GET SM policies return empty list when ism config index does not exist #1072
Conversation
…ig index does not exist Signed-off-by: aggarwalShivani <[email protected]>
Signed-off-by: aggarwalShivani <[email protected]>
...nsearch/indexmanagement/snapshotmanagement/api/transport/get/TransportGetSMPoliciesAction.kt
Outdated
Show resolved
Hide resolved
…elper.unwrapCause Signed-off-by: aggarwalShivani <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1072 +/- ##
============================================
+ Coverage 74.86% 74.90% +0.04%
- Complexity 2808 2814 +6
============================================
Files 367 367
Lines 16518 16522 +4
Branches 2362 2363 +1
============================================
+ Hits 12366 12376 +10
+ Misses 2853 2843 -10
- Partials 1299 1303 +4 ☔ View full report in Codecov by Sentry. |
@bowenlan-amzn would like to confirm with you, return empty list meet your expectation as well |
@Hailong-am Yes, this is the behavior of ISM. Though rollup and transform doesn't catch IndexNotFoundException |
Hi reviewers, i. Docker Security Test Workflow
Analysis - This looks like an issue with security plugin, unrelated to the changes made in this PR. Any thoughts on this? ii. In both these cases, Test and Build Workflow / test-and-build-linux (21, ism) and Test and Build Workflow / test-and-build-linux (21, non-ism), all failures have the same exception ->
I have run some of these testcases individually on my local setup, and they passed. Any pointers on the root cause here? Are they expected? iii. codecov/patch failed with warning - Added lines #L78 - L79 were not covered by tests. |
we can ignore jdk 21 failure, it's known issue. Reference opensearch-project/flow-framework#426 |
Signed-off-by: aggarwalShivani <[email protected]>
67292ca
into
opensearch-project:main
Thanks @Hailong-am and @bowenlan-amzn for the inputs and help here :) |
#1072) (#1078) (cherry picked from commit 67292ca) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: bowenlan-amzn <[email protected]>
Fix for GET request on SM policies to return empty list when ism config index does not exist
Issue #, if available: #1055
Description of changes:
When a GET request is issued to snapshot mananagement API endpoint, to list all available sm policies, it throws an IndexNotFound exception if the ism config index is not yet created. (Note - This index gets created only after atleast one ism/sm policy gets created).
To align the behaviour to ism policies endpoint (that responds with an empty list for missing config index cases), the change is done here for SM policies.
In success case, searchResponse is returned like done before.
In the failure case (for missing config index), instead of throwing OpenSearchStatusException, an empty list of 0 Length is returned from the getAllPolicies function.
If any other exception is caught, then that exception is thrown as-is.
CheckList:
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.