-
Notifications
You must be signed in to change notification settings - Fork 918
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
Remove minimum constraint on OpenSearch hosts to allow empty host and refactor status API to not include OpenSearch status when host is empty #4701
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4701 +/- ##
=======================================
Coverage 66.30% 66.30%
=======================================
Files 3322 3322
Lines 63919 63923 +4
Branches 10115 10117 +2
=======================================
+ Hits 42381 42384 +3
Misses 19050 19050
- Partials 2488 2489 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
61e10d7
to
ce08007
Compare
ab5b588
to
3b512ea
Compare
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.
lgtm
3b512ea
to
70c87cb
Compare
202d8ba
to
64c8a50
Compare
src/core/server/opensearch/status.ts
Outdated
@@ -77,7 +77,7 @@ export const calculateStatus$ = ( | |||
|
|||
return { | |||
level: ServiceStatusLevels.available, | |||
summary: `OpenSearch is available`, | |||
summary: message ?? `OpenSearch is available`, |
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.
summary: message ?? `OpenSearch is available`, | |
summary: message || `OpenSearch is available`, |
||
is a safer operator to guard against ''
and false
.
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.
Though, i would prefer to be stricter..
summary: message ?? `OpenSearch is available`, | |
summary: (message ?? `OpenSearch is available`) || `Unknown`, |
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.
Good idea! Refactored to be more stricter.
Signed-off-by: Bandini Bhopi <bandinib@amazon.com>
64c8a50
to
862d2ca
Compare
Signed-off-by: Bandini Bhopi <bandinib@amazon.com> (cherry picked from commit 8e1ec99) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
Signed-off-by: Bandini Bhopi <bandinib@amazon.com> (cherry picked from commit 8e1ec99) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Josh Romero <rmerqg@amazon.com>
Description
Please note no opensearch host is only considered when explicitly configured. That means you need to add
opensearch.hosts: []
inopensearch_dashboards.yml
file. Default value will remain same which ishttp://localhost:9200
in caseopensearch.hosts
is commented inopensearch_dashboards.yml
file.Issues Resolved
#4700
Screenshot
Status page when
#opensearch.hosts: ["http://localhost:9200"]
ORopensearch.hosts: ["http://localhost:9200"]
inopensearch_dashboards.yml
file.Status page when
opensearch.hosts:[]
inopensearch_dashboards.yml
file.API Status response:
Testing the changes
Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr