[ML] Switching to new datafeed preview #101780
Conversation
jgowdyelastic
added
:ml
auto-backport
|
Pinging @elastic/ml-ui (:ml) |
| } catch (error) { | ||
| // search may fail if the job doesn't already exist | ||
| } |
There was a problem hiding this comment.
do we want to check for a particular error code/message here? otherwise it might silently fail unintentionally
There was a problem hiding this comment.
Failures here will cause failures later on which will be caught by the calling function and displayed to the user in a toast.
I also am not sure what errors we'd need to check for and accept.
This is also a feature that isn't used that much. It has been broken for a long time and no one has noticed.
| } catch (error) { | ||
| // jobs may not exist as this might be called from the AD job wizards | ||
| } |
There was a problem hiding this comment.
do we want to check for a particular error code/message here? otherwise, it might silently fail unintentionally.
| export const datafeedPreviewSchema = schema.object({ | ||
| job: schema.maybe(schema.object(anomalyDetectionJobSchema)), | ||
| datafeed: schema.maybe(datafeedConfigSchema), | ||
| datafeedId: schema.maybe(schema.string()), | ||
| }); |
There was a problem hiding this comment.
I think you need a custom validation callback for this schema because you have mutually exclusive properties.
There was a problem hiding this comment.
Do you know of an example I can copy?
There was a problem hiding this comment.
the previous version using schema.oneOf i think was ok, but it caused other type errors
There was a problem hiding this comment.
found this one
There was a problem hiding this comment.
@jgowdyelastic yes, something like this
|
|
||
| setPreviewJsonString(JSON.stringify(data, null, 2)); | ||
| const { datafeed_config: datafeed, ...job } = combinedJob; | ||
| const preview = await datafeedPreview(undefined, job, datafeed); |
There was a problem hiding this comment.
Before adding a detector, the flyout now shows this. Maybe it would be better not to run the preview before adding a detector? The previous approach showed docs with timestamps.
There was a problem hiding this comment.
I've added a check for the detectors list. if it is empty the preview isn't run and and a message is shown.
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
💔 Backport failed
To backport manually run: |
* [ML] Switching to new datafeed preview * fixing wizard test button * adding schema validator * fixing tests * adding check for empty detectors list Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # x-pack/plugins/ml/public/application/jobs/components/custom_url_editor/utils.js
* [ML] Switching to new datafeed preview * fixing wizard test button * adding schema validator * fixing tests * adding check for empty detectors list Co-authored-by: Kibana Machine <[email protected]>
All datafeed previews in the UI now use the elasticsearch datafeed preview endpoint.
Related to elastic/elasticsearch#70836
Also fixes a recently introduced bug where the Test button when adding a custom url to a job which doesn't exist or hasn't been run will throw an error.
Checklist
Delete any items that are not applicable to this PR.
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
This was checked for breaking API changes and was labeled appropriately