-
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
[ML] Fixes to error handling for analytics jobs and file data viz #60249
[ML] Fixes to error handling for analytics jobs and file data viz #60249
Conversation
Pinging @elastic/ml-ui (:ml) |
...n/data_frame_analytics/pages/analytics_management/components/analytics_list/expanded_row.tsx
Outdated
Show resolved
Hide resolved
@@ -39,12 +39,25 @@ export interface CreateAnalyticsFormProps { | |||
state: State; | |||
} | |||
|
|||
interface ErrorResponse { |
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.
question: do we have a generic interface for the error response?
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.
Added new ErrorResponse
interface in d8926e2
...on/datavisualizer/file_based/components/file_datavisualizer_view/file_datavisualizer_view.js
Outdated
Show resolved
Hide resolved
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.
Added a nit pick, but it's not important.
LGTM
items: Object.entries(stateValues).map(s => { | ||
return { title: s[0].toString(), description: getItemDescription(s[1]) }; | ||
items: Object.entries(stateValues).map(([stateKey, stateValue]) => { | ||
if (stateKey.toString() === 'state') { |
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.
complete nit, stateKey.toString()
is called 3 times in here. it could be pulled out into a title
var at the top of the function.
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.
Done in d3df262
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
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 ⚡️
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* master: (30 commits) [TSVB] fix text color when using custom background color (elastic#60261) Fix import to timefilter from in TSVB (elastic#60296) [NP] Get rid of usage redirectWhenMissing service (elastic#59777) [SIEM] Fix Timeline footer styling (elastic#59587) [ML] Fixes to error handling for analytics jobs and file data viz (elastic#60249) Give better stack traces for Unhandled Promise Rejection warnings (elastic#60235) resolves elastic#58905 (elastic#60120) Added variables button for text fields in Pagerduty component. (elastic#60189) adds test that action vars are rendered for alert action parms (elastic#60310) Closes 59786 by removing the update toast (elastic#60172) [EPM] Packages list tabs (elastic#60167) Added message variables button for Webhook body form field (elastic#60174) Revert "adds new test (elastic#60064)" [Maps] move MapSavedObject type out of telemetry (elastic#60127) [Reporting] Fix error handling for job handler in route (elastic#60161) [Endpoint] TEST: verify alerts page header says 'Alerts' (elastic#60206) EMT-248: implement ack resource to accept event payload to acknowledge agent actions (elastic#60218) Migrate dual validated range (elastic#59689) Embeddable triggers (elastic#58440) [Endpoint] Sample data generator CLI script (elastic#59952) ...
Summary
Fixes handling of errors in the creation of data frame analytics jobs, and the upload in the file data visualizer to provide more details to the user on the error that has occured.
Previously the message part of the error response received from the server was not getting displayed in the UI.
Example from before of permissions error in creation of analytics job:
And after:
and example of fix in file data visualizer upload error:
Also fixes display in the
EuiBadge
tooltip of the failure reason in the data frame analytics job list (previously tooltip was not displaying the failure reason), and adds anEuiBadge
for the display of the job state to highlight errors in the expanded row:Checklist