[ML] Transforms: Migrate client plugin to NP. #59443

walterra · 2020-03-05T16:02:10Z

Summary

Part of #50902.

Migrates the client side plugin of transforms to NP.

Gets rid of the last parts of the shim (http, documentation links)
Moves the plugin from x-pack/legacy/plugins/transform/public to x-pack/plugins/transform
Creates a custom mock for appDependencies based on NP services
Fixes jest tests to get rid of all act() related warnings

Not for kibana-design team: You got pinged here but SCSS files were only moved, not edited.

Checklist

Unit or functional tests were updated or added to match the most common scenarios
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-03-05T16:02:13Z

Pinging @elastic/ml-ui (:ml)

joshdover

Should licensing affect whether or not this Management app is displayed? I don't see any licensing checking code here.

joshdover · 2020-03-05T21:56:57Z

x-pack/plugins/transform/public/plugin.ts

+import { textService } from './app/services/text';
+
+export interface PluginsDependencies {
+  data: any;


FYI you can get the type for this from src/plugins/data/public

Thanks, fixed in abefad6.

joshdover · 2020-03-05T21:58:51Z

x-pack/plugins/transform/public/plugin.ts

+          const startServices = await getStartServices();
+          const [core, plugins] = startServices;
+          const { chrome, docLinks, uiSettings, savedObjects, overlays } = core;
+          const { data } = plugins as PluginsDependencies;


FYI if you add a type parameter to CoreSetup on line 23, you can remove the typecast on line 40: CoreSetup<PluginDependencies>

👍 Fixed in 893275b.

walterra · 2020-03-06T09:42:37Z

@joshdover I copied the behavior from the snapshot_restore plugin to be in line with other plugins in the Elasticsearch section on the management page: The menu item will always be shown but you'll get an error message on the page itself:

That code was already in place and no changes were needed for this cut-over PR.

peteharverson · 2020-03-06T11:39:54Z

x-pack/legacy/plugins/transform/index.ts

-import { resolve } from 'path';
-
-import { PLUGIN } from './common/constants';
-
 export function transform(kibana: any) {


@joshdover is this index.ts file still needed after cutover to x-pack/plugins?

Maybe @sebelga can also chime in on this one. I based that off the structure of the index_management plugin which also has this one file left in legacy.

I had to leave that file in the legacy folder so dependant plugins still in the legacy world that read the configuration would not complain.

If there aren't any plugin the the "legacy" folder that read the transform plugin config, this file should not be needed.

peteharverson · 2020-03-06T12:00:29Z

When running with a base path, I see the following error:

walterra · 2020-03-06T12:04:27Z

@peteharverson basePath issue fixed in d759ac9.

peteharverson

Tested and LGTM

jgowdyelastic

Added a comment, but on the whole LGTM

jgowdyelastic · 2020-03-06T16:13:58Z

x-pack/plugins/transform/public/app/app_dependencies.tsx

+  savedObjects: CoreStart['savedObjects'];
+  overlays: CoreStart['overlays'];
+}
+
 let DependenciesContext: React.Context<AppDependencies>;

 const setAppDependencies = (deps: AppDependencies) => {
  const legacyBasePath = {


this doesn't appear to be being used other than to set basePath in the dependency cache, which could be just deps.http.basePath

jgowdyelastic · 2020-03-06T16:15:13Z

x-pack/plugins/transform/public/app/app_dependencies.tsx

-    autocomplete: deps.plugins.data.autocomplete,
-    docLinks: deps.core.docLinks,
+    autocomplete: deps.data.autocomplete,
+    docLinks: deps.docLinks,
    basePath: legacyBasePath as any,


could this be deps.http.basePath?

walterra · 2020-03-06T16:29:14Z

@jgowdyelastic Thanks for the review! That legacy stuff for setDependenciesCache() is there because I'm reusing KqlQueryBar from the ML plugin. We're hopefully migrating soon to the now available kql query bar from kibana itself so this code will be gone ... so i'm leaving it as is in this PR.

kibanamachine · 2020-03-06T18:41:01Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: d759ac9

History

💚 Build #31397 succeeded d759ac9
💚 Build #31388 succeeded 3c44a7c
💚 Build #31359 succeeded 893275b
💚 Build #31160 succeeded 2ff4463
💔 Build #31136 failed 5a07ca7

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

Migrates the client side plugin of transforms to NP. - Gets rid of the last parts of the shim (http, documentation links) - Moves the plugin from x-pack/legacy/plugins/transform/public to x-pack/plugins/transform - Creates a custom mock for appDependencies based on NP services - Fixes jest tests to get rid of all act() related warnings

* master: (154 commits) Add an optional authentication mode for HTTP resources (elastic#58589) Implement embeddable drilldown menu options (elastic#59232) [Alerting] "Create alert" graph visualization design improvements (elastic#59399) Alerting update route throttle property is missing (elastic#59580) [SIEM] Adds 'Load prebuilt rules' Cypress test (elastic#59529) Show error if field is not found during filter rendering (elastic#59298) Navigate back to discover app during test, because the saved search from the preceding test has major performance problems when used with this test (elastic#59571) Check for alert dialog when doing a force logout (elastic#59329) ensure fs deletes are not cwd dependent (elastic#59570) Empty message for APM service map (elastic#59518) [Drilldowns] <ActionWizard/> Component (elastic#59032) [Reporting] Improve the page exit error messages (elastic#59351) Ensure logged out starting state for tests that need it (elastic#59322) Hide input value from kbn-config-schema error messages (elastic#58843) [ML] Transforms: Migrate client plugin to NP. (elastic#59443) [ML] Disable failing functional tests [SIEM] Update Timeline to use the latest euiFlyoutBody style (elastic#59524) Temporarily remove the project mappings for PR labels (elastic#59493) [Alerting] replace index threshold graph usage of watcher APIs with new API (elastic#59385) [ML] Show view series link in anomalies table for machine_learning_user role (elastic#59549) ...

walterra added 5 commits March 5, 2020 16:45

[ML] Migrate documentation, http to NP.

d871efb

[ML] Deprecate uiMetrics for now.

d8217ce

[ML] Move files from legacy to NP folder.

9238f07

[ML] Working client NP plugin.

7246934

[ML] Fix jest tests.

5a07ca7

walterra added :ml refactoring v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Transforms ML transforms Feature:NP Migration v7.7.0 labels Mar 5, 2020

walterra requested review from pgayvallet, joshdover, darnautov, alvarezmelissa87, peteharverson and jgowdyelastic March 5, 2020 16:02

walterra requested review from a team as code owners March 5, 2020 16:02

walterra self-assigned this Mar 5, 2020

[ML] Fix dependencies.

2ff4463

joshdover reviewed Mar 5, 2020

View reviewed changes

walterra added 3 commits March 6, 2020 09:44

[ML] Fix data plugin types.

abefad6

Merge branch 'master' into ml-transform-client-np

6e97c31

[ML] Tweak plugin setup types.

893275b

Merge branch 'master' into ml-transform-client-np

3c44a7c

walterra requested a review from sebelga March 6, 2020 11:40

peteharverson reviewed Mar 6, 2020

View reviewed changes

[ML] Fix http routes.

d759ac9

peteharverson approved these changes Mar 6, 2020

View reviewed changes

jgowdyelastic approved these changes Mar 6, 2020

View reviewed changes

walterra merged commit f4f956d into elastic:master Mar 6, 2020

walterra deleted the ml-transform-client-np branch March 6, 2020 16:44

This was referenced Mar 6, 2020

[7.x] [ML] Transforms: Migrate client plugin to NP. (#59443) #59568

Merged

[ML] Transforms: New platform migration #50902

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Transforms: Migrate client plugin to NP. #59443

[ML] Transforms: Migrate client plugin to NP. #59443

walterra commented Mar 5, 2020 •

edited

Loading

elasticmachine commented Mar 5, 2020

joshdover left a comment

joshdover Mar 5, 2020

walterra Mar 6, 2020

joshdover Mar 5, 2020

walterra Mar 6, 2020

walterra commented Mar 6, 2020

peteharverson Mar 6, 2020

walterra Mar 6, 2020

sebelga Mar 13, 2020

peteharverson commented Mar 6, 2020

walterra commented Mar 6, 2020

peteharverson left a comment

jgowdyelastic left a comment

jgowdyelastic Mar 6, 2020

jgowdyelastic Mar 6, 2020

walterra commented Mar 6, 2020

kibanamachine commented Mar 6, 2020

[ML] Transforms: Migrate client plugin to NP. #59443

[ML] Transforms: Migrate client plugin to NP. #59443

Conversation

walterra commented Mar 5, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Mar 5, 2020

joshdover left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra commented Mar 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peteharverson commented Mar 6, 2020

walterra commented Mar 6, 2020

peteharverson left a comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra commented Mar 6, 2020

kibanamachine commented Mar 6, 2020

💚 Build Succeeded

History

walterra commented Mar 5, 2020 •

edited

Loading