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] Transforms: Fixes missing number of transform nodes and error reporting in stats bar. #93956

Merged
merged 26 commits into from
Mar 16, 2021

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Mar 8, 2021

Summary

Fixes #92227

  • Adds a Kibana API endpoint transforms/_nodes
  • Adds number of nodes to the stats bar in the transforms list.
  • Shows a callout when no transform nodes are available.
  • Disable all actions except delete when no transform nodes are available.
  • Disables the create button when no transform nodes are available. e970178

image

To test this, add the following to elasticsearch.yml of your single node cluster (that's the default node roles taken from here and the transform role commented out):

node.roles: [
  master,
  data,
  data_content,
  data_hot,
  data_warm,
  data_cold,
  ingest,
  ml,
  remote_cluster_client,
  # transform
]

Checklist

For maintainers

@walterra walterra changed the title [ML] Transforms: Adds number of transform nodes to stats bar. [ML] Transforms: Fixes missing number of transform nodes and error reporting in stats bar. Mar 8, 2021
@walterra walterra added bug Fixes for quality problems that affect the customer experience release_note:fix labels Mar 8, 2021
@walterra walterra marked this pull request as ready for review March 9, 2021 16:38
@walterra walterra requested review from a team as code owners March 9, 2021 16:38
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Unblocking Core's CODEOWNERS review

});

let count = 0;
if (typeof body.nodes === 'object') {
Copy link
Member

Choose a reason for hiding this comment

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

Would this ever be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, I copied that over from older ML code. Updated in bc3e236.

@walterra
Copy link
Contributor Author

Update in bc3e236:

  • API integration test
  • Improved the initial loading behavior for the stats bar and transform list
  • Fixed types for the API endpoint

isPopulatedObject(node) &&
{}.hasOwnProperty.call(node, 'attributes') &&
isPopulatedObject(node.attributes) &&
{}.hasOwnProperty.call(node.attributes, 'transform.node')
Copy link
Member

Choose a reason for hiding this comment

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

i think this check is too strict, transform.node may be missing entirely from one of the nodes, in which case the whole check will fail and count will be returned as 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch and thinking about multiple nodes! I fixed the type guard and added a unit test in cf49a77.

@walterra
Copy link
Contributor Author

@elasticmachine merge upstream

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 latest edits and LGTM

@walterra
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
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.

LGTM

@walterra
Copy link
Contributor Author

The tests caught something I missed: The code was based on the legacy format to identify transform nodes via attributes. I changed it to be based on roles instead in 9a471ee. More details in this ES issue: elastic/elasticsearch#69260 (comment)

@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
transform 946.6KB 949.8KB +3.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 432.2KB 432.3KB +62.0B

History

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

cc @walterra

@walterra walterra merged commit f3b74b4 into elastic:master Mar 16, 2021
@walterra walterra deleted the ml-transform-nodes branch March 16, 2021 10:41
walterra added a commit to walterra/kibana that referenced this pull request Mar 16, 2021
…porting in stats bar. (elastic#93956)

- Adds a Kibana API endpoint transforms/_nodes
- Adds number of nodes to the stats bar in the transforms list.
- Shows a callout when no transform nodes are available.
- Disable all actions except delete when no transform nodes are available.
- Disables the create button when no transform nodes are available.
walterra added a commit that referenced this pull request Mar 16, 2021
…porting in stats bar. (#93956) (#94677)

- Adds a Kibana API endpoint transforms/_nodes
- Adds number of nodes to the stats bar in the transforms list.
- Shows a callout when no transform nodes are available.
- Disable all actions except delete when no transform nodes are available.
- Disables the create button when no transform nodes are available.
walterra added a commit to walterra/kibana that referenced this pull request Mar 30, 2021
…porting in stats bar. (elastic#93956)

- Adds a Kibana API endpoint transforms/_nodes
- Adds number of nodes to the stats bar in the transforms list.
- Shows a callout when no transform nodes are available.
- Disable all actions except delete when no transform nodes are available.
- Disables the create button when no transform nodes are available.
# Conflicts:
#	x-pack/plugins/transform/common/api_schemas/type_guards.ts
walterra added a commit that referenced this pull request Mar 31, 2021
…rror reporting in stats bar. (#93956) (#95754)

- Adds a Kibana API endpoint transforms/_nodes
- Adds number of nodes to the stats bar in the transforms list.
- Shows a callout when no transform nodes are available.
- Disable all actions except delete when no transform nodes are available.
- Disables the create button when no transform nodes are available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Transforms ML transforms :ml release_note:fix v7.12.1 v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Transforms: List should highlight when transform role is missing
7 participants