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

[MDS] Refactor several utility functions in data source plugin #6417

Closed
huyaboo opened this issue Apr 11, 2024 · 5 comments
Closed

[MDS] Refactor several utility functions in data source plugin #6417

huyaboo opened this issue Apr 11, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request multiple datasource multiple datasource project v2.16.0

Comments

@huyaboo
Copy link
Member

huyaboo commented Apr 11, 2024

Is your feature request related to a problem? Please describe.
Currently, there are two functions being repeated in plugins that can simply be extracted into data source plugin utility functions:

  • A function that decides whether to use local cluster or remote OpenSearch client based on data source id
  • A function that retrieves the data source id based on data source title/name

Describe the solution you'd like
There should be three utility functions placed in the data source plugin that will be exposed so that other plugins can consume:

  • decideClient(context: RequestHandlerContext, dataSourceId?: string, withLongNumeralsSupport: boolean): return the correct client
  • decideLegacyClient(context: RequestHandlerContext, dataSourceId?: string): return the correct legacy client
  • fetchDataSourceNameById(client: SavedObjectsClientCommon, dataSourceTitle: string): return the data source id (if it exists)
    • SavedObjectClientWrapper should be defined in both public and server folders for passing type checks

Describe alternatives you've considered

N/A

Additional context

N/A

@huyaboo huyaboo added the enhancement New feature or request label Apr 11, 2024
This was referenced Apr 11, 2024
@seraphjiang seraphjiang added the multiple datasource multiple datasource project label Apr 13, 2024
@bandinib-amzn
Copy link
Member

Thank you for continuous improvement and insist on high standards.

@BionIT
Copy link
Collaborator

BionIT commented Jun 4, 2024

@huyaboo Should we target this for 2.16 instead?

@BionIT BionIT added v2.16.0 and removed v2.15.0 labels Jun 4, 2024
@BionIT BionIT assigned zhyuanqi, zhongnansu and huyaboo and unassigned huyaboo and zhyuanqi Jun 13, 2024
@zhongnansu
Copy link
Member

Unlike "decideClient" that will be used by many internal and external plugins, where plugins need to choose the correct client to talk to either local cluster, or data source cluster, for "fetchDataSourceIdByName", I don't think it's necessary to make it a util in data source plugin, because

  1. it's only being used in vega and timeline plugin, I am not aware of any other internal or external plugins that needs it.
  2. there's context specific logic in the existing fetchDataSourceIdByName function, see below for example. "data_source_name" is very specific to the timeline/vega expression use case. So it doesn't make sense to make it general

throw new Error(
`Expected exactly 1 result for data_source_name "${config.data_source_name}" but got ${possibleDataSourceIds.length} results`
);
}

cc: @huyaboo @BionIT

@BionIT
Copy link
Collaborator

BionIT commented Jul 30, 2024

Hi @zhongnansu Is this complete? Can we close the issue?

@zhongnansu
Copy link
Member

Hi @zhongnansu Is this complete? Can we close the issue?

Yes, closing

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.16.0
Projects
None yet
Development

No branches or pull requests

6 participants