Skip to content

[ML] Transforms: Deprecate custom KibanaContext.#59133

Merged
walterra merged 5 commits intoelastic:masterfrom
walterra:ml-transform-kibana-context-cleanup
Mar 4, 2020
Merged

[ML] Transforms: Deprecate custom KibanaContext.#59133
walterra merged 5 commits intoelastic:masterfrom
walterra:ml-transform-kibana-context-cleanup

Conversation

@walterra
Copy link
Copy Markdown
Contributor

@walterra walterra commented Mar 3, 2020

Summary

  • Deprecates the custom KibanaContext.
  • Where applicable dependencies provided via KibanaContext are now passed on via AppDependencies.
  • The main feature of KibanaContext was to populate index pattern and saved search information for the transform wizard. This is now provided via the useSearchItems() custom hook.

Checklist

For maintainers

@walterra walterra requested a review from a team as a code owner March 3, 2020 12:04
@walterra walterra self-assigned this Mar 3, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Copy Markdown
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

Code LGTM

Copy link
Copy Markdown
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
Copy Markdown
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 LGTM

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

History

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

@walterra walterra merged commit 29975fa into elastic:master Mar 4, 2020
@walterra walterra deleted the ml-transform-kibana-context-cleanup branch March 4, 2020 15:56
@lukeelmers
Copy link
Copy Markdown
Contributor

Looks like this PR adds quite a bit to some of the snapshots... Specifically it includes individual services from the app providers in the snapshot.

This means that whenever someone changes one of the services (even in a non-breaking way like adding a new API), the test will break even if it isn't relying on that service. This just happened to me in #58805.

We've had this pop up a few different plugins recently, so I figured I should bring it to your attention. You might consider whether snapshots are needed here, or whether you can snapshot a smaller subset of the component tree.

spalger added a commit that referenced this pull request Mar 4, 2020
@spalger
Copy link
Copy Markdown
Contributor

spalger commented Mar 4, 2020

@walterra This started causing failures on master so it was reverted, please resubmit. https://kibana-ci.elastic.co/job/elastic+kibana+master/3417/testReport/

@spalger spalger added backport:skip This PR does not require backporting reverted labels Mar 4, 2020
@lukeelmers lukeelmers mentioned this pull request Mar 4, 2020
20 tasks
@walterra
Copy link
Copy Markdown
Contributor Author

walterra commented Mar 5, 2020

@spalger @lukeelmers Thanks for taking care of this while I was away. I agree those snapshots will cause trouble. I'll come up with a different approach in the follow-up PR.

jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 5, 2020
…re/files-and-filetree

* 'master' of github.com:elastic/kibana: (254 commits)
  Convert discover_page to ts, remove redundunt methods (elastic#59312)
  [Fix for Vis Editor] Revert setting time field to empty string when it's undefined (elastic#58873)
  Delete legacy search endpoint (elastic#59341)
  [Uptime] Improve duration chart (elastic#58404)
  [Snapshot & Restore] NP migration (elastic#59109)
  [ML] Add support for date_nanos time field in anomaly job wizard (elastic#59017)
  Revert "Makes alerting and actions optional properties for interface RequestH… (elastic#59264)"
  Change remote_clusters ID to remoteClusters (elastic#59246)
  Makes alerting and actions optional properties for interface RequestH… (elastic#59264)
  Clean up date histogram agg type. (elastic#58805)
  [ML] Management: fix license unsubscribe (elastic#59365)
  Remove documentation for server.cors settings (elastic#59096)
  Edit alert flyout (elastic#58964)
  [SIEM] Fix rule delete/duplicate actions (elastic#59306)
  move mouse to close obstructing tooltip (elastic#59214)
  Reset page after deleting (elastic#59310)
  Make sure phrases input filter triggers autosuggestons (elastic#59299)
  Add loading count source for http requests (elastic#59245)
  Revert "[ML] Transforms: Deprecate custom KibanaContext. (elastic#59133)"
  Expose metrics service to public API (elastic#59294)
  ...

# Conflicts:
#	src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:NP Migration Feature:Transforms Transforms :ml refactoring release_note:skip Skip the PR/issue when compiling release notes reverted v7.7.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants