-
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] Fix incorrect behaviors for Anomaly Detection jobs when resetting or converting to advanced job #90078
Conversation
Pinging @elastic/ml-ui (:ml) |
const hasDatafeed = | ||
typeof job.datafeed_config === 'object' && Object.keys(job.datafeed_config).length > 0; | ||
const datafeedId = hasDatafeed ? job.datafeed_config.datafeed_id : ''; | ||
const hasDatafeed = typeof datafeed === 'object' && Object.keys(datafeed).length > 0; |
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.
datafeed
should be set to undefined
like job
is on line 377.
This should also happen here:
mlJobService.tempJobCloningObjects.job = undefined; |
Sorry, I missed this on the previous PR.
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.
Updated here d867b46
jenkins test this |
I'm seeing some strange behavior when resetting a multi metric, or population job, after it has run. Sometimes it goes into the Single metric wizard, and sometimes it's gone into the Advanced wizard. For example, with this config on
Update: I no longer see this issue after the edits in b8de989 |
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 no longer see the issues with Reset job after the changes in b8de989.
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.
Tested latest edit and LGTM
saveNewJob(job: any): Promise<any>; | ||
cloneDatafeed(datafeed: any): Datafeed; | ||
saveNewJob(job: Job): Promise<any>; | ||
cloneDatafeed(Datafeed: Datafeed | undefined): Datafeed; |
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.
this function shouldn't take undefined
as it doesn't know how to deal with it.
we need a check here to ensure datafeed
is not undefined
:
if (mlJobService.tempJobCloningObjects.job !== undefined) { |
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.
Updated here 0fa0b09
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 SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
Summary
This PR fixes a bug introduced by #88898 which causes datafeed_config to be appended back to the job when resetting or converting to advanced job.
Before