Skip to content
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 deletion of models that are not used by pipelines #114107

Merged

Conversation

peteharverson
Copy link
Contributor

@peteharverson peteharverson commented Oct 6, 2021

Summary

Fixes #113013 where trained models that weren't in use by a pipeline could not be selected for deletion in the models list in the data frame analytics page as the delete action and selection checkboxes were incorrectly disabled.

image

This was caused by a regression introduced in #99174, which meant that an empty pipelines object is returned by the /api/ml/trained_models/:modelId endpoint for models not used by pipelines, where before null was returned.

Also adds extra functional tests for the trained models list, which would have caught the original bug, and fixes an error in use_delete_action.tsx in the return value for userCanDelete.

Checklist

Fixes #113013

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the UI fix and it worked fine.
Great to see the functional tests coming! 🎉
Left a few comments.

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ⚡

@peteharverson peteharverson self-assigned this Oct 7, 2021
@kibanamachine
Copy link
Contributor

kibanamachine commented Oct 7, 2021

⏳ Build in-progress, with failures

Failed CI Steps

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @peteharverson

@pheyos
Copy link
Member

pheyos commented Oct 7, 2021

Checking test stability in a flaky test runner job ... no failures in 50 runs ✔️

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 3.6MB 3.6MB +576.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @peteharverson

@peteharverson peteharverson added the auto-backport Deprecated - use backport:version if exact versions are needed label Oct 7, 2021
@peteharverson peteharverson merged commit 0b14195 into elastic:master Oct 7, 2021
@peteharverson peteharverson deleted the ml-trained-models-delete-fix branch October 7, 2021 20:08
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 7, 2021
…14107)

* [ML] Fix deletion of models that are not used by pipelines

* [ML] Edits from review

* [ML] Fix jest test for index switch in delete job modal

* [ML] Fix API test calls to createTestTrainedModels

* [ML] Remove unnecessary async from jest test
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Oct 7, 2021
…114355)

* [ML] Fix deletion of models that are not used by pipelines

* [ML] Edits from review

* [ML] Fix jest test for index switch in delete job modal

* [ML] Fix API test calls to createTestTrainedModels

* [ML] Remove unnecessary async from jest test

Co-authored-by: Pete Harverson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Data Frame Analytics ML data frame analytics features :ml release_note:fix review v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Unable to select/delete data frame analytics models via UI in 7.15.0
6 participants