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.

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

[Multiple Datasource] With default data source rendered in the data source component, we need to let consumers know which default data source id is used #6276

Closed
2 tasks done
Tracked by #5872
BionIT opened this issue Mar 27, 2024 · 9 comments
Assignees
Labels
enhancement New feature or request multiple datasource multiple datasource project v2.14.0

Comments

@BionIT
Copy link
Collaborator

BionIT commented Mar 27, 2024

Is your feature request related to a problem? Please describe.

In #6237 and #6186, we introduced new feature to setting default data source as well as to make sure there is a default data source. The default data source will be the default option in context where data source is needed. This introduces some complexity in the state management since only the data source component knows about the default data source and renders it as default option, we need to pass the default data source id back to the consumers

Describe the solution you'd like

We can use the callback function passed by the consumers at the first render when the default data source is identified.

Describe alternatives you've considered

Show empty page until user changes the data source selector

Additional context

Add any other context or screenshots about the feature request here.

Tasks:

@BionIT BionIT added the enhancement New feature or request label Mar 27, 2024
@BionIT BionIT added multiple datasource multiple datasource project v2.14.0 labels Mar 27, 2024
@BionIT BionIT changed the title [Multiple Datasource] With default data source we need to let consumers know which default data source id is used [Multiple Datasource] With default data source rendered in the data source component, we need to let consumers know which default data source id is used Mar 27, 2024
@RamakrishnaChilaka
Copy link

@BionIT, letting the consumer know via callback approach will result in two renders, and the backend APIs will be called from the first render already which we want to avoid, as it is unnecessary traffic.

If it is possible, we can expose a function in the core, which says what is the default data source id or the default data source is, we can initialise the default state in our constructor to that. If the CX gave no default data source, you initialise it to Local Cluster.

I am not sure how can we handle if hideLocalCluster prop as true, but the dataSourceId passed to the component is empty..

@BionIT
Copy link
Collaborator Author

BionIT commented Mar 27, 2024

@RamakrishnaChilaka Based on discussion, the plugin page which consumes the component, should render the data source picker first, and the picker will have the logic to determine appropriate default option and use the callback function to inform the page, the rest of page should wait until the initial value is retrieved before rendering, once the value is received, then render the rest of page

The default/initial option provided back by the picker would have 3 possible states with default data source introduced and no local cluster scenario:

  1. undefined - this means there is no default data source, no local cluster, and the page should not make any external requests, and the page should render no results (we need to check how it would look like)
  2. empty data source id {id:'', label: 'Local cluster'} - this means there is no default data source, but has local cluster, and the page should make request to local cluster
  3. data source id {id:'', label: ''} - this means there is a data source being recognized as default by the picker logic, and the page should make request to the remote data source using the data source id

@RamakrishnaChilaka
Copy link

Thanks Lu. @BionIT, can you provide a utility function that takes a dataSourceId as input and returns whether the plugin code can be mounted? Because it's better to not have context inside Plugin code that when dataSourceId is undefined, plugin will mount its body and we will repeat this logic across all plugins, which is not desirable.

or provide with an additional boolean field in dataSourcecallBackFunc like canMountPluginComponent, when it's true we will mount the rest of the plugin code, otherwise we will render Opensearch loader or some other component based on the UI/UX feedback.

@kavilla
Copy link
Member

kavilla commented Apr 2, 2024

Without diving into the original implementation. I have a few questions.

@BionIT, does this work in the same way that default index patterns work? The initial data source will be the first data source? Does it get saved into the config (UI Settings) saved object? For example like:

Screenshot 2024-04-01 at 6 56 22 PM

If not then I think we should follow the existing method of adding the UI setting for this? This setting can be added if data sources is enabled, and if the default data source is null then maybe that can be the indication that the user has data sources enabled and we render it.

Also we can consider adding the default data source to add to the app state _a so on load we do a request to get the default data source if enabled and then populate the app state with it and if the plugin needs checks the app state and sees a defined data source id then it will get render that logic so.

Basically mimicking the index pattern, here we can see the app state that includes the index pattern id:

https://playground.opensearch.org/app/data-explorer/discover#?_a=(discover:(columns:!(_source),isDirty:!f,sort:!()),metadata:(indexPattern:ff959d40-b880-11e8-a6d9-e546fe2bba5f,...

Please ignore if this does not actually relate to the issue you are trying to solve.

@kavilla
Copy link
Member

kavilla commented Apr 2, 2024

Will assign you @BionIT since you open the issue as it seems like a RFC. If this is incorrect please unassign but will remove the Untriage label.

@BionIT
Copy link
Collaborator Author

BionIT commented Apr 2, 2024

Thanks @kavilla ! @zhyuanqi is working on it, and default data source is implemented similarly as index pattern inside uiSetting, see implementations in #6237 and #6186 . The complexity is default data source might not be the default option shown in the data source component since it might be an incompatible data source which based on requirement should not be shown. Thus the default data source set by user might not be the default option shown to the user, and we need to let plugin know since plugin pages make requests to backend to fetch results based on selected data source on initial page load - cc @kgcreative

@BionIT BionIT removed their assignment Apr 2, 2024
@zhyuanqi
Copy link
Collaborator

zhyuanqi commented Apr 3, 2024

working on it.

@BionIT
Copy link
Collaborator Author

BionIT commented Apr 3, 2024

Thanks @zhyuanqi !!!

@BionIT BionIT mentioned this issue Apr 3, 2024
7 tasks
@BionIT
Copy link
Collaborator Author

BionIT commented Apr 5, 2024

2 PRs have been merged, closing the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request multiple datasource multiple datasource project v2.14.0
Projects
None yet
Development

No branches or pull requests

4 participants