-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Stack Monitoring] update setup mode data when required #113934
Conversation
Pinging @elastic/stack-monitoring (Team:Monitoring) |
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
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.
Looks good! 👏
I've tried also to enable Metricbeat monitoring from the Elasticsearch nodes pages (it should work similar as it works for kibana instances) but there is an error when entering the setup mode there. I logged a separate ticket to look into this -> #114076
requests.push(updateSetupModeData()); | ||
} | ||
|
||
Promise.allSettled(requests).then((results) => { |
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.
I think this should be .catch
because if I'm not missing anything here we don't have to do anything with results, we only want to process errors. (Although I'm working on the errors page, so I can also change it after you merge it to master)
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.
allSettled never rejects. Thought it would be appropriate here since requests don't look related and individual error handling may be a good option, but I'll leave it for the tasks dedicated to that :)
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.
Didn't notice the allSettled
🙃, I'll work on it in my PR
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Summary
Context here #113873
Testing
api/monitoring/v1/setup/collection/cluster/:id
was polled in setup mode