-
Notifications
You must be signed in to change notification settings - Fork 913
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
Clean up stale augment-vis
saved objs
#4059
Clean up stale augment-vis
saved objs
#4059
Conversation
src/plugins/visualize/public/application/components/visualize_listing.tsx
Outdated
Show resolved
Hide resolved
Thanks for the detailed proposal @ashwin-pc . Agreed this is a better strategy and removes the side-effect way of using the augment vis loader within the visualization loader's delete(). I've finished making those changes you suggested - I had to take a dependency on uiActions in a few places, but I figure that's for the best anyways so it can be used in the future. I also wonder if uiActions belongs in core, because it seems to look that way... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good - thanks @ashwin-pc for the suggestion, I like this approach, and I agree that we may want to make UIActions even more accessible via core in the future.
I do want to verify the ability to delete non-visualization objects like visbuilder. Other than that, the only other blocker is the commented out sections.
src/plugins/visualize/public/application/components/visualize_listing.tsx
Outdated
Show resolved
Hide resolved
src/plugins/saved_objects_management/public/triggers/saved_object_delete_trigger.ts
Outdated
Show resolved
Hide resolved
@@ -32,7 +32,7 @@ import { SavedObjectLoader } from '../../../saved_objects/public'; | |||
|
|||
export interface SavedObjectsManagementServiceRegistryEntry { | |||
id: string; | |||
service: SavedObjectLoader; | |||
service: SavedObjectLoader | any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does unknown
also work here?
src/plugins/vis_augmenter/public/actions/plugin_resource_delete_action.ts
Outdated
Show resolved
Hide resolved
src/plugins/vis_augmenter/public/actions/plugin_resource_delete_action.ts
Outdated
Show resolved
Hide resolved
src/plugins/vis_augmenter/public/actions/saved_object_delete_action.ts
Outdated
Show resolved
Hide resolved
@ohltyler There's also one failing unit test:
|
@joshuarrrr addressed comments, fixed test. |
...plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx
Outdated
Show resolved
Hide resolved
src/plugins/saved_objects_management/public/register_services.ts
Outdated
Show resolved
Hide resolved
@@ -32,7 +32,7 @@ import { SavedObjectLoader } from '../../../saved_objects/public'; | |||
|
|||
export interface SavedObjectsManagementServiceRegistryEntry { | |||
id: string; | |||
service: SavedObjectLoader; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, Why do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
src/plugins/visualize/public/application/components/visualize_listing.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
34c2a21
to
67ae9ab
Compare
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
src/plugins/saved_objects_management/public/register_services.ts
Outdated
Show resolved
Hide resolved
...plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! thanks for making those changes :)
Description
This PR adds cleanup of stale/outdated/dangling
augment-vis
saved objs.These objs can become "stale" if either (1) the associated (referenced) visualization has been deleted, or (2) the associated plugin resource has been deleted.
Cleaning up after deleted plugin resources
In the render workflow of a visualization, when fetching all of it's
augment-vis
saved objs, if any include a deleted plugin resource, delete it. See the new helper fncleanupStaleObjects()
. DEMO: see Figure 1 below.Cleaning up after deleted visualizations
We update the vis saved object loader to implement a custom
delete()
fn which will fetch allaugment-vis
saved objs that have this vis id and delete it. To make sure this loader function is called, we update a few places where deletion of a visualization may commonly occur:visualization
type). DEMO: see Figure 3 below.Saved objects management plugin: table pageTODO: this may not be done as part of this effort. A lot of edge cases and batch processing scenarios we need to consider. Is also more of an edge caseAdditional info
RESOURCE_DELETED
error type such that plugins can propagate this information to indicate their resource has been deleted. This is required in order to be picked up incleanupStaleObjects()
to get cleaned up.Demo videos
Figure 1: Given 2 stale
augment-vis
saved objects with deleted plugin resources, when re-rendering the visualization that is associated 2 these 2 objects, they are cleaned up and deleted in the background. Confirmed via.kibana
indexstale-plugin-resource-demo.webm
Figure 2: Given a visualization with 1
augment-vis
saved object, when deleting the visualization fromvisualize
plugin, theaugment-vis
saved object is cleaned up in the background. Confirmed via.kibana
indexstale-vis-visualize-plugin-demo.webm
Figure 3: Given a visualization with 1
augment-vis
saved object, when deleting the visualization from saved objects management plugin's edit object page, theaugment-vis
saved object is cleaned up in the background. Confirmed via.kibana
indexstale-vis-saved-obj-edit-demo.webm
Issues Resolved
Closes #4044
Testing the changes
Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr