-
Notifications
You must be signed in to change notification settings - Fork 915
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
Datasource id is required if multiple datasource is enabled and no default cluster supported #5751
Datasource id is required if multiple datasource is enabled and no default cluster supported #5751
Conversation
…fault cluster supported Signed-off-by: Xinrui Bai <[email protected]>
Signed-off-by: Xinrui Bai <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #5751 +/- ##
=======================================
Coverage 66.96% 66.97%
=======================================
Files 3301 3301
Lines 63434 63437 +3
Branches 10104 10107 +3
=======================================
+ Hits 42480 42487 +7
+ Misses 18502 18498 -4
Partials 2452 2452
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Left some comments about the feature flag
src/plugins/data/server/search/opensearch_search/decide_client.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/server/search/opensearch_search/decide_client.ts
Outdated
Show resolved
Hide resolved
src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.ts
Outdated
Show resolved
Hide resolved
I see this issue is introduced when config option added for hide/show local cluster option in data source picker.
|
Signed-off-by: Xinrui Bai <[email protected]>
Thanks for the comments, I create a quick one pager, let's review it first~ |
Signed-off-by: Xinrui Bai <[email protected]>
Based on the one pager, this change is for server error handling, and able to decoupled from other UX changes. Also this change will in main branch, not target to 2.12. Thanks |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-5751-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 bbd40e105089f4efe75c7bd94ab9231dbbb93580
# Push it to GitHub
git push --set-upstream origin backport/backport-5751-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x Then, create a pull request where the |
Not required for 2.12 @xinruiba will you be able to create a manual backport PR? |
…fault cluster supported (#5751) * datasource id is required if multiple datasource is enabled and no default cluster supported Signed-off-by: Xinrui Bai <[email protected]> --------- Signed-off-by: Xinrui Bai <[email protected]> (cherry picked from commit bbd40e1) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…fault cluster supported (#5751) (#5842) * datasource id is required if multiple datasource is enabled and no default cluster supported --------- (cherry picked from commit bbd40e1) Signed-off-by: Xinrui Bai <[email protected]> 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>
Description
When datasource_id is empty and default cluster not supported, the search returns 500 error which is unexpected since this is more of input error. This change is to check if multiple datasource enabled and default cluster not supported, when search with a empty datasource_id then the request is invalid and should return code 400.
Issues Resolved
#5754
#5699
Testing the changes
Send empty datasource_id in the payload with datasource enabled and defaultCluster disabled in the yaml config returns 400 instead of 500
Send payload with no datasource_id in the payload with datasource enabled and defaultCluster disabled in the yaml config returns 400 instead of 500
Send payload with invalid datasource_id
Check List
yarn test:jest
yarn test:jest_integration