[Canvas] Migrate by value embeddables #123515

cqliu1 · 2022-01-20T22:18:41Z

Summary

Closes #101639.

This runs all of the by-value embeddables on a Canvas workpad through the embeddable migrations.

Checklist

Delete any items that are not applicable to this PR.

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2022-01-27T20:19:53Z

Pinging @elastic/kibana-presentation (Team:Presentation)

cqliu1 · 2022-02-06T03:46:09Z

@elasticmachine merge upstream

kibanamachine · 2022-02-06T03:46:22Z

expected head sha didn’t match current head ref.

Check for id in embeddable input Removed unused import Fixed tests Fix variable name Move migration into embeddable function definition Remove unused code

crob611

One little question but other than that looks and works good for me 👍

crob611 · 2022-02-07T15:32:28Z

x-pack/plugins/canvas/canvas_plugin_src/functions/external/embeddable.ts

+      (state: ExpressionAstFunction): ExpressionAstFunction => {
+        const embeddableInput = decode(state.arguments.config[0] as string);
+
+        if (embeddableInput.attributes || embeddableInput.savedVis) {


What's the need for this check? Is there a reason we wouldn't want to always run the input through the migration function?

I think we only need to handle by-value embeddable on our end since migration for the saved objects that back the by-reference embeddables is handled elsewhere, so it might be duplicating work. But I can remove the check. It's possible that the embeddable input stored in the expression for by-reference embeddables could have attributes that need to be migrated, so it probably doesn't hurt to run all of them through the embedddable migrations.

cqliu1 · 2022-02-08T21:59:23Z

@elasticmachine merge upstream

…a into canvas/embeddable-migration

kibana-ci · 2022-02-09T07:46:09Z

💚 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
`canvas`	1.0MB	1.0MB	+267.0B

History

💔 Build #22635 failed 790a186
💔 Build #22539 failed 108b5d6
💔 Build #22173 failed 7468446
💔 Build #22004 failed 6cd9e276493f0c16b1abc1bb43ab4d102fb8aef4
💔 Build #21977 failed a8d4a37eeea4a66166b0d64dfba187bda592a3a4

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

* Add migrations for by value embeddables Check for id in embeddable input Removed unused import Fixed tests Fix variable name Move migration into embeddable function definition Remove unused code * Cleanup * Fix embeddable test * Remove check for by-value embeddables in embeddable function migration * Removed unused import Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 8e8c4b8)

cqliu1 requested a review from a team as a code owner January 20, 2022 22:18

cqliu1 force-pushed the canvas/embeddable-migration branch from 3138b60 to eac4c36 Compare January 24, 2022 21:44

cqliu1 added chore Feature:Canvas release_note:skip Skip the PR/issue when compiling release notes v8.1.0 labels Jan 27, 2022

kibanamachine added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Jan 27, 2022

cqliu1 added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort labels Jan 27, 2022

cqliu1 force-pushed the canvas/embeddable-migration branch 2 times, most recently from ff23ebd to 1c822a6 Compare January 31, 2022 19:29

cqliu1 force-pushed the canvas/embeddable-migration branch 2 times, most recently from 1951328 to 8cde82f Compare February 4, 2022 21:43

cqliu1 added 2 commits February 7, 2022 09:18

Add migrations for by value embeddables

3ce46d8

Check for id in embeddable input Removed unused import Fixed tests Fix variable name Move migration into embeddable function definition Remove unused code

Cleanup

7468446

cqliu1 force-pushed the canvas/embeddable-migration branch from 6cd9e27 to 7468446 Compare February 7, 2022 16:18

Fix embeddable test

108d793

crob611 approved these changes Feb 8, 2022

View reviewed changes

Remove check for by-value embeddables in embeddable function migration

108b5d6

kibanamachine and others added 3 commits February 8, 2022 16:59

Merge branch 'main' into canvas/embeddable-migration

790a186

Removed unused import

d8e119c

Merge branch 'canvas/embeddable-migration' of github.com:cqliu1/kiban…

ee77933

…a into canvas/embeddable-migration

cqliu1 merged commit 8e8c4b8 into elastic:main Feb 9, 2022

kibanamachine added the backport:skip This commit does not require backporting label Feb 9, 2022

cqliu1 deleted the canvas/embeddable-migration branch February 9, 2022 17:11

cqliu1 added the v8.2.0 label Feb 9, 2022

cqliu1 mentioned this pull request Feb 9, 2022

[8.1] [Canvas] Migrate by value embeddables (#123515) #125121

Merged

cqliu1 restored the canvas/embeddable-migration branch June 8, 2022 13:57

This was referenced Mar 9, 2023

canvas should register expression functions in setup life cycle #106511

Closed

[canvas] expressions should be migrated and references extracted/injected #107494

Closed

nreese mentioned this pull request Aug 21, 2024

[canvas][maps] by-value map embeddables have broken layers #190994

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Canvas] Migrate by value embeddables #123515

[Canvas] Migrate by value embeddables #123515

cqliu1 commented Jan 20, 2022 •

edited

Loading

elasticmachine commented Jan 27, 2022

cqliu1 commented Feb 6, 2022

kibanamachine commented Feb 6, 2022

crob611 left a comment

crob611 Feb 7, 2022

cqliu1 Feb 8, 2022

cqliu1 commented Feb 8, 2022

kibana-ci commented Feb 9, 2022

[Canvas] Migrate by value embeddables #123515

[Canvas] Migrate by value embeddables #123515

Conversation

cqliu1 commented Jan 20, 2022 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

elasticmachine commented Jan 27, 2022

cqliu1 commented Feb 6, 2022

kibanamachine commented Feb 6, 2022

crob611 left a comment

Choose a reason for hiding this comment

crob611 Feb 7, 2022

Choose a reason for hiding this comment

cqliu1 Feb 8, 2022

Choose a reason for hiding this comment

cqliu1 commented Feb 8, 2022

kibana-ci commented Feb 9, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

History

cqliu1 commented Jan 20, 2022 •

edited

Loading