-
Notifications
You must be signed in to change notification settings - Fork 891
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
Vis-Builder Integration with Data Explorer Proposal #5407
Comments
Great proposal @ananzh! Definitely more inclined to option one. Here are a few suggestions to that.
Thanks for the detailed description of the migration! |
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
Prefixing the sliceTo address the challenge of managing states in a centralized system with different applications like VisBuilder and DataExplorer, while avoiding naming conflicts and ensuring easy access to the states, we propose the following steps: Prefix State Keys with view.idPrefixing state keys with the view.id to avoid naming conflicts. Then we can maintain the original state names within each view (like style, visualization, and ui in vis builder) but prepend the view's identifier when integrating them into the RootState. The only exception is metadata, because it is duplicate as the one in RootState. State naming in Data ExplorerThe register slice id in Data Explorer store (src/plugins/data_explorer/public/utils/state_management/preload.ts):
Therefore in plugin.ts, each view needs to register a proper ID. If there are multiple slices in one view, the slice id would be
There are four slices: ui, style, editor, visualization. The registered slice names in Data Explorer are vis-builder-ui, vis-builder-style, vis-builder-editor and vis-builder-visualization. For view with single state, then the easiest naming is to have state name equal to PLUGIN_ID. For example, in Discover, both registered view ID and state is Dynamic State AccessTo access these namespaced states easily, we can create a selector function that dynamically constructs the state key based on the view's ID. For example, within VisBuilder, we use a generic selector that automatically appends vis-builder- to the state keys. Here are some reference helper functions.
To use
To use:
Dispatch Mechanism in Centralized State ManagementIn the centralized state management system, where different slices of state are prefixed with their respective view IDs (like vis-builder-editor for the editor slice), we actually don't need to modify the actions:
|
How will this work with auto complete and typescript types and suggestions? today selecting using the selector is typesafe |
@ashwin-pc the above is more general discussion on how to migrate/work with a centralized state management store. In vis-builder, we will add one helper function
To use it:
The Type Safety: Ensures that only valid keys for the VisBuilder's state are used, reducing the risk of runtime errors due to incorrect state keys. Autocomplete: When using this selector function, developers will receive autocomplete suggestions for the stateKey parameter, which can speed up development and reduce errors. Error Detection: If a developer tries to use an invalid key, TypeScript will flag it as an error, catching potential issues early in the development process. The types defined in Data Explorer is more general to fit more views. But VisBuilder and other views could maintain type safety across the entire application. |
@ashwin-pc I updated the previous comment. For |
Oh I like it if the helper exists. I just don't want to have to remember how to get to the state value without auto complete 😂 |
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
This PR completes Task 1 and 2 in opensearch-project#5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve opensearch-project#5492 opensearch-project#5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: opensearch-project#5522 Signed-off-by: ananzh <[email protected]> [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
…#6591) This PR completes Task 1 and 2 in #5407. * Reconstruct and allow VisBuilder to be rendered from DataExplorer * Follow proposal task 2 option 1 to migrate state management to DataExplorer Issue Resolve #5492 #5493 [2][VisBuilder Migration] Add context and implement side panel * add useVisBuilderContext * modify preloadedState in Data Explorer * implement side panel Issue Resolve: #5522 [3][VisBuilder Migration] Combine components into VisBuilderCanvas Signed-off-by: ananzh <[email protected]>
Overview
The Data Explorer project has taken a step forward in streamlining the data exploration experience in OpenSearch Dashboards (OSD) by addressing security concerns and offering a unified platform. As a strategic move, it's essential to integrate the vis-builder plugin with Data Explorer to provide a comprehensive data analysis framework for users.
Vis-builder has been instrumental in visual data exploration, offering capabilities like chart creation and advanced visualization settings. Its incorporation into Data Explorer would enhance the platform's overall data analytics and visualization potential.
There are two stages of the work:
Stage 1: Incorporate the vis-builder plugin as a view inside the Data Explorer.
Goals
Note: This proposal focuses primarily on Stage 1.
Stage 2: Enhancements (currently under design & discussion)
Goals
Note: Not the scope of current proposal.
Objective
To seamlessly incorporate the vis-builder plugin as a view inside the Data Explorer to offer a unified data exploration experience, thereby enriching the data analysis capabilities of OSD.
Tasks
[Task 1] Reconstruct and Reroute VisBuilder Plugin
[Task 2] State Management Migration
There are issues to migrate states. First, simplify metadata slice which should only ffocus on the editor's state.
Second migrate simplified metadata slice and other 3 slices, there are two proposals.
Proposal 1: Allowing Data Explorer to Register Multiple Slices for a Single View
Objective
Retain the modularity of individual slices for each component of the Vis Builder while simplifying integration with the Data Explorer.
Steps
Handle side effects: In the Data Explorer store, we are dynamically adding slices (reducers) for each view. In the visBuilder, there are two side effects: handlerEditorState and handlerParentAggs. These side effects are listeners that perform specific actions when certain changes in the state occur. The current Data Explorer does not have these side effects integrated, which leads us to the main challenge. However, we see there is a TODO in Data Explore store config
Add Side effects here to apply after changes to the store are made. None for now.
, which indicates an initial design considerations on how integrate view’s side effect. Based on this, here are stepshandleChange
should iterate through each registered view and execute its side effects if they exist.Some cleaning work: Clean out redux store from VisBuilder and utilize the one from Data Explorer.
Pros and Cons
Proposal 2: Combining VisBuilder Slices and register one slice to Data Explorer
This proposal defines a combined VisBuilderState which encompasses all the individual sub-states. Data Explorer current setting is one slice/view. A view, for example Discover, needs to specify a slice like below and data explorer register a slice via registerSlice.
Different from Discover, VisBuilder has multiple slices. Then for consistency and manage purpose, it might be ideal to keep the one slice for one view.
Steps
Dispatch Actions: As mentioned above, dispatch needs to get updated. First take the current action with specific slice, for example setStyleState(newStyleValue) specific to the styleSlice. Then wrap that action with the setStyleState action creator to make it specific to the visBuilderSlice. Then dispatch the resulting action.
Understand and check extraReducers: Since we have combined the slices, it's crucial that extraReducers is moved in the combined slice (VisBuilderSlice) which should know which subsection of the state to modify.
Handle side effects: Similar to Proposal 1.
Pros and Cons
updateStyle: (state, action: PayloadAction<StyleState>) => { state.style = styleReducer(state.style, action); }
[Task 3] Context Provider Migration/Creation
[Task 4] UI modifications
The main part of this task is to divide src/plugins/vis_builder/public/application/app.tsx into a canvas area (VisBuilderCanvas) and a side panel (VisBuilderPanel). The canvas is where users interact with visualizations, and the side panel only provides field selector. Below is the brief re-construction:
VisBuilderPanel should constructed from cleaned LeftNav .
VisBuilderCanvas is the main canvas of VisBuilder. It should have the TopNav at the top, ConfigPanel on the left, Workspace in the middle, and RightNav on the right.
[Task 5] Comprehensive test for VisBuilder
Tentative Timeline
Based on 2-3 devs
Week 1-2
Week 3-4
Week 5-6
Additional Considerations
The text was updated successfully, but these errors were encountered: