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.

Sign up for GitHub

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

Jump to bottom

Migrate visualization, annotation, graph to saved_object_content_storage #168520

Merged
merged 15 commits into from
Oct 14, 2023
Merged

Migrate visualization, annotation, graph to saved_object_content_storage #168520

merged 15 commits into from
Oct 14, 2023

Conversation

drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented Oct 10, 2023 •
edited
Loading

Summary

Close #167421

@drewdaemon drewdaemon force-pushed the 167421/adopt-shared-content-storage branch from 4653713 to 62f3470 Compare October 10, 2023 18:27
@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@drewdaemon drewdaemon changed the title Migrate visualization, annotation, graph to saved_object_content_storage Migrate visualization, annotation, graph to saved_object_content_storage Oct 11, 2023
@drewdaemon drewdaemon changed the title Migrate visualization, annotation, graph to saved_object_content_storage Migrate visualization, annotation, graph to saved_object_content_storage Oct 11, 2023
@drewdaemon drewdaemon marked this pull request as ready for review October 11, 2023 12:05
@drewdaemon drewdaemon requested review from a team as code owners October 11, 2023 12:05
@drewdaemon drewdaemon added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:skip Skip the PR/issue when compiling release notes labels Oct 11, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@drewdaemon
Copy link
Contributor Author

I'm wondering if there's a way to simulate a dev environment in the API integration tests (make initializerContext.env.mode.dev true).

@stratoula stratoula requested a review from Dosant October 11, 2023 12:47
Dosant
Dosant reviewed Oct 11, 2023
View reviewed changes
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Great! Thank you 👏

src/plugins/visualizations/server/content_management/visualization_storage.ts
allowedSavedObjectAttributes: [
'title',
'description',
'version',
Copy link
Contributor

Choose a reason for hiding this comment

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

is version removal expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took this list of props from here. In other words, version wasn't included in the item response before, so I removed it to keep parity with the current API.

Is this the right thing to do?

x-pack/plugins/graph/server/content_management/graph_storage.ts Outdated Show resolved Hide resolved
davismcphee
davismcphee approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Code only review. saved_object_content_storage.ts type change LGTM 👍

@stratoula
Copy link
Contributor

@dej611 you know this area better than me, can you review? 🙏

@drewdaemon drewdaemon requested a review from Dosant October 12, 2023 14:38
dej611
dej611 approved these changes Oct 13, 2023
View reviewed changes
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Tested locally with Safari and all works 👍

@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #54 / Saved objects management feature controls saved objects management global all privileges "after all" hook in "global all privileges"
  • [job] [logs] FTR Configs #54 / Saved objects management feature controls saved objects management global all privileges listing "before all" hook for "shows all saved objects"

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/content-management-utils 126 125 -1
eventAnnotation 200 201 +1
total -0
Unknown metric groups

API count

id before after diff
eventAnnotation 200 201 +1

History

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

@drewdaemon drewdaemon merged commit bab28dc into elastic:main Oct 14, 2023
@kibanamachine kibanamachine added v8.12.0 backport:skip This commit does not require backporting labels Oct 14, 2023
dej611 pushed a commit to dej611/kibana that referenced this pull request Oct 17, 2023
@drewdaemon @kibanamachine @dej611
Migrate visualization, annotation, graph to `saved_object_cont…
6889ac5 
…ent_storage` (elastic#168520)

## Summary

Close elastic#167421

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davismcphee davismcphee davismcphee approved these changes

@dej611 dej611 dej611 approved these changes

@Dosant Dosant Awaiting requested review from Dosant

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CM] Migrate visualization annotation graph to saved_object_content_storage
8 participants
@drewdaemon @elasticmachine @stratoula @kibana-ci @dej611 @Dosant @davismcphee @kibanamachine