[Cases] Mark lens migrations as deferred #159291

cnasikas · 2023-06-08T09:43:11Z

Summary

In Cases, users can add lens visualizations in a case. The lens's attributes persist inside the cases-comments saved object. If the lens team adds a migration a migration will run for the cases-comments SOs to migrate any lens visualization inside the case comment. In this PR we defer the lens migrations to be done on read when it is fetched by users. This way, the cases comments will not be migrated at Kibana startup.

Blocker for #158468

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

cnasikas · 2023-06-08T09:52:02Z

x-pack/plugins/cases/server/saved_object_types/migrations/comments.ts

+  return {
+    // @ts-expect-error: remove when core changes the types
+    deferred,
+    transform: (


Same function as before. Just set it to the transform attribute.

elasticmachine · 2023-06-09T08:45:51Z

Pinging @elastic/response-ops (Team:ResponseOps)

elasticmachine · 2023-06-09T08:45:53Z

Pinging @elastic/response-ops-cases (Feature:Cases)

cnasikas · 2023-06-09T08:50:35Z

x-pack/plugins/cases/server/saved_object_types/migrations/comments.ts

-
-      const migratedMarkdown = { ...parsedComment, children: migratedComment };
+  return {
+    // @ts-expect-error: remove when core changes the types


@rudolf Is it ok to set it as true when needed?

yes, until #152807 is merged there's a caveat: if there's partial updates against this saved object it could cause migrations to run twice (update comment without updating typeVersion) or never run (updating typeVersion without updating comment).

Looking at https://github.com/elastic/kibana/blob/main/x-pack/plugins/cases/common/api/cases/comment/index.ts#L250C5-L261 it doesn't seem like we allow partial updates?

Otherwise we're blocked on #152807

Yes, this is correct. We do not allow partial updates for comments.

rudolf · 2023-06-09T09:35:20Z

x-pack/plugins/cases/server/saved_object_types/migrations/comments.ts

-      });
-
-      const migratedMarkdown = { ...parsedComment, children: migratedComment };
+  return {


We now change "normal" lens migrations into deferred ones. This would work, but I think it'd be more explicit if the actual lens migration was also forced to set deferred: true.

To do this we'd need to handle migrate being migrate: MigrateFunction | SavedObjectMigrationParams

And then do something like:

if(deferred) { // nit: I would call this variable shouldDefer if(typeof(migrate) == "function" || migrate.deferred === false) { // just for illustration, typescript won't like this throw new Error("Cases only allows deferred migrations starting in 8.9.0") } }

Discussed this with @cnasikas and @stratoula

This would require a small refactor in embeddable migration framework to allow for MigrateFunction | SavedObjectMigrationParams instead of just MigrateFunction. While we ultimately want all embeddable migrations to be deferred we're likely going to start moving to a client-side embeddable migration framework soon #158677

So for now we decided we won't force the lens migration to be deferred: true but only apply that when we migrate the case comments.

rudolf · 2023-06-09T09:54:34Z

x-pack/plugins/cases/server/saved_object_types/migrations/comments.ts

-
-      const migratedMarkdown = { ...parsedComment, children: migratedComment };
+  return {
+    // @ts-expect-error: remove when core changes the types


yes, until #152807 is merged there's a caveat: if there's partial updates against this saved object it could cause migrations to run twice (update comment without updating typeVersion) or never run (updating typeVersion without updating comment).

Looking at https://github.com/elastic/kibana/blob/main/x-pack/plugins/cases/common/api/cases/comment/index.ts#L250C5-L261 it doesn't seem like we allow partial updates?

Otherwise we're blocked on #152807

x-pack/plugins/cases/server/saved_object_types/migrations/comments.test.ts

cnasikas · 2023-06-09T14:52:56Z

@elasticmachine merge upstream

… into lens_deferred_migrations

kibana-ci · 2023-06-09T16:51:19Z

💚 Build Succeeded

Buildkite Build
Commit: 1bba41a

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	413	417	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	497	501	+4
total			+6

History

💛 Build #134048 was flaky a103f85

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

cc @cnasikas

## Summary In Cases, users can add lens visualizations in a case. The lens's attributes persist inside the `cases-comments` saved object. If the lens team adds a migration a migration will run for the `cases-comments` SOs to migrate any lens visualization inside the case comment. In this PR we defer the lens migrations to be done on read when it is fetched by users. This way, the cases comments will not be migrated at Kibana startup. Blocker for elastic#158468 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Mark lens migrations as deferred

5143120

cnasikas added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.9.0 labels Jun 8, 2023

cnasikas requested review from rudolf and stratoula June 8, 2023 09:43

cnasikas self-assigned this Jun 8, 2023

cnasikas commented Jun 8, 2023

View reviewed changes

cnasikas marked this pull request as ready for review June 9, 2023 08:45

cnasikas requested a review from a team as a code owner June 9, 2023 08:45

cnasikas commented Jun 9, 2023

View reviewed changes

rudolf reviewed Jun 9, 2023

View reviewed changes

rudolf approved these changes Jun 9, 2023

View reviewed changes

adcoelho reviewed Jun 9, 2023

View reviewed changes

x-pack/plugins/cases/server/saved_object_types/migrations/comments.test.ts Show resolved Hide resolved

adcoelho reviewed Jun 9, 2023

View reviewed changes

x-pack/plugins/cases/server/saved_object_types/migrations/comments.test.ts Outdated Show resolved Hide resolved

adcoelho approved these changes Jun 9, 2023

View reviewed changes

kibanamachine and others added 4 commits June 9, 2023 10:52

Merge branch 'main' into lens_deferred_migrations

a103f85

PR feedback

638f4ad

Merge branch 'lens_deferred_migrations' of github.com:cnasikas/kibana…

23e265e

… into lens_deferred_migrations

Fix

1bba41a

cnasikas enabled auto-merge (squash) June 9, 2023 16:18

cnasikas merged commit d2f00a1 into elastic:main Jun 9, 2023

cnasikas deleted the lens_deferred_migrations branch June 9, 2023 16:51

kibanamachine added the backport:skip This commit does not require backporting label Jun 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cases] Mark lens migrations as deferred #159291

[Cases] Mark lens migrations as deferred #159291

cnasikas commented Jun 8, 2023 •

edited

Loading

cnasikas Jun 8, 2023

elasticmachine commented Jun 9, 2023

elasticmachine commented Jun 9, 2023

cnasikas Jun 9, 2023

rudolf Jun 9, 2023

cnasikas Jun 9, 2023

rudolf Jun 9, 2023

rudolf Jun 9, 2023

rudolf Jun 9, 2023

cnasikas commented Jun 9, 2023

kibana-ci commented Jun 9, 2023

ESLint disabled line counts

Total ESLint disabled count

[Cases] Mark lens migrations as deferred #159291

[Cases] Mark lens migrations as deferred #159291

Conversation

cnasikas commented Jun 8, 2023 • edited Loading

Summary

Checklist

For maintainers

cnasikas Jun 8, 2023

Choose a reason for hiding this comment

elasticmachine commented Jun 9, 2023

elasticmachine commented Jun 9, 2023

cnasikas Jun 9, 2023

Choose a reason for hiding this comment

rudolf Jun 9, 2023

Choose a reason for hiding this comment

cnasikas Jun 9, 2023

Choose a reason for hiding this comment

rudolf Jun 9, 2023

Choose a reason for hiding this comment

rudolf Jun 9, 2023

Choose a reason for hiding this comment

rudolf Jun 9, 2023

Choose a reason for hiding this comment

cnasikas commented Jun 9, 2023

kibana-ci commented Jun 9, 2023

💚 Build Succeeded

Metrics [docs]

ESLint disabled line counts

Total ESLint disabled count

History

cnasikas commented Jun 8, 2023 •

edited

Loading