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] Update transform cloning to include description and new fields #78364

Merged
merged 11 commits into from
Oct 1, 2020

Conversation

qn895
Copy link
Member

@qn895 qn895 commented Sep 23, 2020

Summary

Fixes #75264. Cloning a transform should now carry over the description, frequency, settings.max_page_search_size, and settings.docs_per_sec.

cloning_transform_2

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and looks good.

Looks like the cloning transform functional test needs an edit too as the description is now cloned. I think it will be worth adding another test in, checking that the advanced properties (frequency and max page search size) are cloned as expected.

@qn895 qn895 requested a review from pheyos September 24, 2020 18:28
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM with extra functional tests added.

@walterra
Copy link
Contributor

It would be great if you can add some updated tests for getCreateTransformRequestBody() and getCreateTransformSettingsRequestBody() in request.test.ts to consider the new settings.

@qn895
Copy link
Member Author

qn895 commented Sep 30, 2020

It would be great if you can add some updated tests for getCreateTransformRequestBody() and getCreateTransformSettingsRequestBody() in request.test.ts to consider the new settings.

@walterra @pheyos I updated the tests here b82513e and I think this PR is ready for a final review 🙏 Thanks

@pheyos
Copy link
Member

pheyos commented Oct 1, 2020

Checked test stability in a flaky test runner job: no failure 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.

Functional test changes LGTM

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Latest changes LGTM!

…ne-fields

# Conflicts:
#	x-pack/test/functional/apps/ml/settings/index.ts
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
transform 1.2MB 1.2MB +1.5KB

History

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

@qn895 qn895 merged commit fd7dd41 into elastic:master Oct 1, 2020
@qn895 qn895 deleted the fix-transform-clone-fields branch October 1, 2020 17:41
phillipb added a commit to phillipb/kibana that referenced this pull request Oct 1, 2020
…aly-detection-partition-field

* 'master' of github.com:elastic/kibana: (76 commits)
  Fix z-index of KQL Suggestions dropdown (elastic#79184)
  [babel] remove unused/unneeded babel plugins (elastic#79173)
  [Search] Fix timeout upgrade link (elastic#79045)
  Always Show Embeddable Panel Header in Edit Mode (elastic#79152)
  [Ingest]: add more test for transform index (elastic#79154)
  [ML] DF Analytics: Collapsable sections on results pages (elastic#76641)
  [Fleet] Fix agent policy change action migration (elastic#79046)
  [Ingest Manager] Match package spec `dataset`->`data_stream` and `config_templates`->`policy_templates` renaming (elastic#78699)
  Revert "[Metrics UI] Add ability to override datafeeds and job config for partition field (elastic#78875)"
  [ML] Update transform cloning to include description and new fields (elastic#78364)
  chore(NA): remove non existing plugin paths from case api integration tests (elastic#79127)
  [Ingest Manager] Ensure we trigger agent policy updated event when we bump revision. (elastic#78836)
  [Metrics UI] Display No Data context.values as [NO DATA] (elastic#78038)
  [Monitoring] Missing data alert (elastic#78208)
  [Lens] Fix embeddable title and description for reporting and dashboard tooltip (elastic#78767)
  [Lens] Consistent Drag and Drop styles (elastic#78674)
  [ML] Model management UI fixes and enhancements (elastic#79072)
  [Metrics UI] Add ability to override datafeeds and job config for partition field (elastic#78875)
  [Security Solution]Fix basepath used by endpoint telemetry tests (elastic#79027)
  update rum agent version which contains longtasks (elastic#79105)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Transforms: Cloning transform should retain description
6 participants