[ML] Transforms: Move ML "Data Frame Transforms" to Kibana management section "Transforms". #45880

walterra · 2019-09-17T10:50:15Z

Summary

Part of #45352.

Moves "Data frame transforms" from the ML plugin to its own "transform" plugin within the Kibana management section.

Review hints:

The new plugins lives in x-pack/legacy/plugins/transform.
The overall wrapping code of the plugin is based on the snapshot/restore-plugin. This includes the plugin setup, shims, react-router, cluster priviliges checks, breadcrumbs.
snapshot/restores cluster privileges checks have been extended with the capabilities approach (e.g. canCreateTransform) used in ML. The cluster privileges checks are used to check general access to sections (like TransformManagement or CreateTransform). The capabilities are used to enable/disable actions within sections (e.g. Create transform button on transform list page).
References to data frame/data_frame/dataFrame etc. have been removed or renamed to transform*.
Data frame transform related menu items have been removed from the ML plugin.
Some code pieces like required utilities or previously shared components have been copied from ML code (e.g. StatsBar, JobIcon).
pages from the ML plugin have been renamed to sections.
UI Metric constants are defined for client based telemetry based on the snapshot/restore plugin, but the actual tracking will be implemented in a follow up.
DocumentationLinksService and DocTitleService have been brought over from snapshot/restore but are not in use yet.
There's now httpService brought over from snapshot/restore which is used for license and privileges checks based on snapshot/restore code. However, there's also http_service.ts brought over from ML to handle the transform API. This should be unified/cleaned-up in a follow up.
x-pack/legacy/plugins/transform/public/shared_imports.ts is used to reference cross-plugin imports (some brought over as is from snapshot/restore, some from ML plugin)

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
~~Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
~~This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-09-26T00:09:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

… learning.

alisonelizabeth

nice work @walterra!

I took a quick look through the code and overall LGTM. I mainly reviewed the UX and noted some things that I think could be changed to better align with our other ES management apps. (Suggestions only.)

Transforms table view

Keep all app content within EuiPageContent.
Create/reload buttons we generally keep to the right of the search bar
Nice to have: link to the transform documentation
Table bulk actions we generally use EuiContextMenu
Example:
Row actions we generally only render an icon with tooltip
Example:
For the stats bar, consider using EuiPanel and EuiStat components.

Some similar examples in our existing apps:
The icon to expand rows is generally within the Actions column. This seems to be the case within the EUI table documentation example as well.

Transforms empty prompt

I think you could probably remove the horizontal rule at the top and change the link to a button.

Example:

Create transforms wizard

I think it might make sense to put the content within EuiPageContent as well. At first, I thought some fields were disabled due to the gray background.
We've tended to use horizontal steps (rather than vertical) in our wizards. Not sure it makes sense to change here, but just wanted to note.
It seemed a little strange to me that we don't redirect back to the list view after creating a new transform. Is there a reason for that?
I ran into this error scenario. Do you think it might make more sense to render a callout on the page and maintain the wizard contents?

Example:

Errors while testing

I noticed this error when starting kibana:

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 21 change listeners added. Use emitter.setMaxListeners() to increase limit

It looks like the same error mentioned here: #46448 (comment)

I also noticed this warning in the browser console when entering the transform wizard:

Warning: State updates from the useState() and useReducer() Hooks don't support the second callback argument. To execute a side effect after rendering, declare it in the component body with useEffect().

alisonelizabeth · 2019-10-08T17:55:02Z

...sections/transform_management/components/create_transform_button/create_transform_button.tsx

+    !capabilities.canStartStopTransform;
+
+  const createTransformButton = (
+    <>


nit: i don't think you need to use fragments here

Fixed in 1b34e00.

alisonelizabeth · 2019-10-08T17:55:45Z

x-pack/legacy/plugins/transform/public/app/services/documentation/documentation_links.ts

+    return `${this.esPluginDocBasePath}${TRANSFORM_DOC_PATHS.plugins}`;
+  }
+
+  public getSnapshotDocUrl() {


i don't think this is being used - copy/paste mistake?

You're right of course! Fixed in 1b34e00.

walterra · 2019-10-09T07:41:21Z

@peteharverson I fixed the width of the JSON tab in expanded rows for both the transform plugin and the data frame analytics jobs list in be3a66c. I'll look into updating the layout in a follow up together with the feedback from Elasticsearch UI team.

elasticmachine · 2019-10-09T08:55:31Z

💚 Build Succeeded

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

elasticmachine · 2019-10-09T09:47:13Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 6b40948

walterra · 2019-10-09T14:20:30Z

@alisonelizabeth I checked with the platform team and pushed a fix for the MaxListenersExceededWarning you were seeing in 2f6848a.

elasticmachine · 2019-10-09T15:17:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 2f6848a

walterra · 2019-10-09T15:20:50Z

@alisonelizabeth I was able to fix the React hooks related error you were seeing in 89d3398 - the root cause was that I passed on a hook's set function as a callback to another component which was called with two arguments, thanks for pointing it out!

peteharverson

Tested latest edits and LGTM

elasticmachine · 2019-10-09T16:24:47Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 89d3398

…ion "Transforms". (elastic#45880) Moves "Data frame transforms" from the ML plugin to its own "transform" plugin within the Kibana management section.

…ion "Transforms". (#45880) (#47745) Moves "Data frame transforms" from the ML plugin to its own "transform" plugin within the Kibana management section.

elasticmachine · 2019-10-22T13:11:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 89d3398

walterra self-assigned this Sep 17, 2019

walterra added v7.5.0 v8.0.0 labels Sep 17, 2019

walterra force-pushed the transform-plugin branch from 11b9b9a to dbc3ecc Compare September 25, 2019 17:08

walterra added 23 commits September 26, 2019 14:22

[transform] Initial plugin setup.

52421c3

[transform] Remove references to 'data frame'.

9ea0c26

[transform] Fix imports and tests.

8aa35c7

[transform] Remove references to 'dataframe'.

2979b26

[transform] Use 'transform' prefix instead of 'ml' in SCSS.

f9cbfd8

[transform] Fix translations.

b04fa4e

[transform] Fix types.

9a043eb

[transform] Remove more data frame references.

c8dac54

[transform] Remove data frames from ml nav tabs.

dedb788

[transform] Move field types to type/fields.

2874429

[transform] Move Kibana api endpoints to plugins/transform.

f56c3b1

[transform] Fix interface usage.

269165c

[transform] Rename pages to sections.

56e2741

[transform] Move permissions checks to transform plugin.

55b0063

[transform] Remove functional navigation transfrom tests from machine…

21fc923

… learning.

[transform] Fix types.

c804b37

[transform] Fix tests.

5f0454e

[transform] Fix tests. Removes deprecated code.

bf144f5

[transform] Removes deprecated functional tests code.

842257b

[transform] Fix translations.

6a9b3a8

[transform] Fix tests.

e0348f0

[transform] Adds source selection modal.

cc7e8c7

[transform] Fix saved searches.

63f0228

walterra force-pushed the transform-plugin branch from 87d3ca7 to 63f0228 Compare September 30, 2019 10:06

elastic deleted a comment from elasticmachine Sep 30, 2019

alisonelizabeth reviewed Oct 8, 2019

View reviewed changes

[transform] Fix EuiTabbedContent width for JSON views.

be3a66c

walterra added 8 commits October 9, 2019 09:49

[transform] Simplify isTransformIdValid/isJobIdValid.

0e12550

[transform] Return early for JobIcon unset message.

ac5cfe7

[transform] Simplify toArray.

b2e5e11

[transform] Simplify StatsBar styles.

7f5320f

[transform] Review feedback: Tweaks plugint.ts.

370a9f2

[transform] Review feedback: Remove Fragment, remove unused code.

1b34e00

[transform] Change EuiHighlight to just strong.

6b40948

[transform] Remove unused code.

304b4c7

walterra added 2 commits October 9, 2019 12:42

Merge branch 'master' into transform-plugin

f35e796

[transform] Increase upstreamPlugin.status.setMaxListeners to 21.

2f6848a

[transform] Review feedback: Fix hooks issue.

89d3398

peteharverson approved these changes Oct 9, 2019

View reviewed changes

walterra merged commit bcf9ec3 into elastic:master Oct 9, 2019

walterra deleted the transform-plugin branch October 9, 2019 18:13

walterra mentioned this pull request Oct 9, 2019

[7.x] [transform] Move ML "Data Frame Transforms" to Kibana management section "Transforms". (#45880) #47745

Merged

walterra changed the title ~~[transform] Move ML "Data Frame Transforms" to Kibana management section "Transforms".~~ [ML] Transforms: Move ML "Data Frame Transforms" to Kibana management section "Transforms". Oct 22, 2019

walterra mentioned this pull request Oct 22, 2019

[ML] Plugin navigation Anomaly Detection / Data Frames #35721

Closed

walterra mentioned this pull request Nov 18, 2019

[ML] Transforms: New platform migration #50902

Closed

39 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Transforms: Move ML "Data Frame Transforms" to Kibana management section "Transforms". #45880

[ML] Transforms: Move ML "Data Frame Transforms" to Kibana management section "Transforms". #45880

walterra commented Sep 17, 2019 •

edited by peteharverson

Loading

elasticmachine commented Sep 26, 2019

alisonelizabeth left a comment •

edited

Loading

alisonelizabeth Oct 8, 2019

walterra Oct 9, 2019

alisonelizabeth Oct 8, 2019

walterra Oct 9, 2019

walterra commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

walterra commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

walterra commented Oct 9, 2019 •

edited

Loading

peteharverson left a comment

elasticmachine commented Oct 9, 2019

elasticmachine commented Oct 22, 2019

[ML] Transforms: Move ML "Data Frame Transforms" to Kibana management section "Transforms". #45880

[ML] Transforms: Move ML "Data Frame Transforms" to Kibana management section "Transforms". #45880

Conversation

walterra commented Sep 17, 2019 • edited by peteharverson Loading

Summary

Checklist

For maintainers

elasticmachine commented Sep 26, 2019

💚 Build Succeeded

alisonelizabeth left a comment • edited Loading

Choose a reason for hiding this comment

alisonelizabeth Oct 8, 2019

Choose a reason for hiding this comment

walterra Oct 9, 2019

Choose a reason for hiding this comment

alisonelizabeth Oct 8, 2019

Choose a reason for hiding this comment

walterra Oct 9, 2019

Choose a reason for hiding this comment

walterra commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

💚 Build Succeeded

elasticmachine commented Oct 9, 2019

💔 Build Failed

walterra commented Oct 9, 2019

elasticmachine commented Oct 9, 2019

💚 Build Succeeded

walterra commented Oct 9, 2019 • edited Loading

peteharverson left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 9, 2019

💚 Build Succeeded

elasticmachine commented Oct 22, 2019

💚 Build Succeeded

walterra commented Sep 17, 2019 •

edited by peteharverson

Loading

alisonelizabeth left a comment •

edited

Loading

walterra commented Oct 9, 2019 •

edited

Loading