Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Adpot Miki's suggestions.

Co-authored-by: Miki <amoo_miki@yahoo.com>
  • Loading branch information
kristenTian and AMoo-Miki committed Nov 2, 2022
1 parent 3a8715f commit 9ed56fd
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions docs/multi-datasource/client_management_design.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ We need to configure the data source client by either creating a new one, or loo
}
```

- Get root client: Look up client Pool by **endpoint**, return client if existed. If it misses, we create new client instance and load into pool. At this step, the client won't have any auth info.
- Get root client: Look up the client pool by **endpoint** and return the client if it exists. If a client was not found, a new client instance is created and loaded into pool. At this step, the client won't have any auth info.

- Get credentials: Call crypto service utilities to **decrypt** user credentials from `DataSource` Object.
- Assemble the actual query client: With auth info and root client, well leverage the `opensearch-js` connection pooling strategy to create the actual query client from root client by `client.child()`.

#### 4.2.1 Legacy Client

OpenSearch Dashboards is forked from Kibana 7.10. At the time of the fork happened, there are 2 types of client used in the codebase. One is the new client, which later was migrated as `opensearch-js`, the other one is the legacy client which is `elasticsearch-js`. Legacy clients are still used many critical features, such as visualization, index pattern management, along with new client.
OpenSearch Dashboards was forked from Kibana 7.10 and at the time, the codebase made two types of clients available for use. One was the "new client" which has since been separated into `opensearch-js`, and the other was the legacy client named `elasticsearch-js`. Legacy clients are still used by some core features like visualization and index pattern management.

```ts
// legacy client
Expand Down Expand Up @@ -174,7 +174,7 @@ This is for plugin to access data source client via request handler. For example

### 4.4 Refactor data plugin search module to call core API to get datasource client

`Search strategy` is the low level API of data plugin search module. It retrieves clients and query OpenSearch. It needs to be refactored to switch between default client and datasource client, depending on whether a request is send to datasource or not.
`Search strategy` is the low level API of data plugin search module. It retrieves clients and queries OpenSearch. It needs to be refactored to switch between the default client and the datasource client, depending on whether or not a request is sent to the datasource.

Currently default client is retrieved by search module of data plugin to interact with OpenSearch by this API call. Ref: [opensearch-search-strategy.ts](https://github.com/opensearch-project/opensearch-dashboards/blob/e3b34df1dea59a253884f6da4e49c3e717d362c9/src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.ts#L75)

Expand All @@ -184,7 +184,7 @@ const client: OpenSearchClient = core.opensearch.client.asCurrentUser;
client.search(params);
```

Similarly well have the following for datasource use case. `AsCurrentUser` is something does not make sense for datasource, because its always thecurrentuser credential defined in thedatasource”, that we are using to create the client, or look up the client pool.
Similarly well have the following for datasource use case. `AsCurrentUser` doesn't really apply to a datasource because it’s always the “current” user's credentials, defined in thedatasource”, that gets used to initialize the client or lookup the client pool.

```ts
if (request.dataSource) {
Expand Down

0 comments on commit 9ed56fd

Please sign in to comment.