[Watcher] Add support for ERROR and UNKNOWN watch action statuses #55092
Conversation
alisonelizabeth
added
Feature:Watcher
v8.0.0
Team:Kibana Management
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
| } | ||
| ) | ||
| ); | ||
| return ACTION_STATES.UNKNOWN; |
There was a problem hiding this comment.
It seemed more appropriate to mark the action in an unknown state, rather than throwing an error and making the watch list view unusable.
The only potential outstanding question is what the overall watch status should return in this scenario. I did not make any code changes to how this is handled, so it currently will return OK. Thoughts?
There was a problem hiding this comment.
I definitely agree with rather keeping the view usable in case of a bad status (i.e., catering for this condition here definitely seems like the correct thing to do). Overall I am happy with this approach 👍
The one advantage of throwing in this case, however, is that we were able to discover this bug which I think may have remained hidden otherwise. So being totally silent also has a cost (as you've noted).
Judging how action_status and watch_status hang together, it would be cool to be able to say that if the last action status was unknown, then the watch_status is unknown? Remaining in this state should be a red flag to the user + they can still use the UI, we should communicate it's a possible bug. This could be a separate unit of work to this fix though!
Let me know what you think? We could revert this line for now and address later, or commit and address later. I'm happy with either for now.
There was a problem hiding this comment.
@jloleysens I like this idea a lot - thanks! I opened #55410 to address separately.
x-pack/legacy/plugins/watcher/server/np_ready/models/action_status/action_status.js
Outdated
Show resolved
Hide resolved
| } | ||
| ) | ||
| ); | ||
| return ACTION_STATES.UNKNOWN; |
There was a problem hiding this comment.
I definitely agree with rather keeping the view usable in case of a bad status (i.e., catering for this condition here definitely seems like the correct thing to do). Overall I am happy with this approach 👍
The one advantage of throwing in this case, however, is that we were able to discover this bug which I think may have remained hidden otherwise. So being totally silent also has a cost (as you've noted).
Judging how action_status and watch_status hang together, it would be cool to be able to say that if the last action status was unknown, then the watch_status is unknown? Remaining in this state should be a red flag to the user + they can still use the UI, we should communicate it's a possible bug. This could be a separate unit of work to this fix though!
Let me know what you think? We could revert this line for now and address later, or commit and address later. I'm happy with either for now.
…atus/action_status.js Co-Authored-By: Jean-Louis Leysens <jloleysens@gmail.com>
|
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…t-of-legacy * 'master' of github.com:elastic/kibana: [Watcher] Add support for additional watch action statuses (elastic#55092) [ML] Fix entity controls update. (elastic#55524) [ML] Fixing ML's watcher integration (elastic#55439) [ML] Fixes permissions checks for data visualizer create job links (elastic#55431) [SearchProfiler] Move out of legacy (elastic#55331) # Conflicts: # x-pack/plugins/watcher/server/models/action_status/action_status.js
cjcenizal
changed the title
Fixes #54730
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Documentation was added for features that require explanation or tutorialsThis was checked for keyboard-only and screenreader accessibilityRelease note
This change fixes a bug in which the UI did not load the watch overview page if one of user's watches had a watch action in an invalid state.