-
Notifications
You must be signed in to change notification settings - Fork 123
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
[PROPOSAL] OpenSearch JS Version 3.0 and OpenSearch Typescript #803
Comments
I love the end goal: an opensearch-js to be rewritten in TypeScript, and to use a code generator.
If I understand correctly the TypeScript client will be completely compatible with opensearch-js 3.0, so it should be a branch and eventually be merged to main as a replacement, then we would be shipping a non-breaking opensearch-js 3.x, but rewritten in TypeScript. If this is not possible, I don't see why we need opensearch-js 3.0 at all. It's a breaking change for users, then they would have another breaking change switching to a TypeScript client, so why cause the pain twice?
I wonder whether we can implement an optional compatibility mode for this that users would inject in the client to preserve backwards compat. |
The breaking changes in 3.0 will only affect a few applications that use those very obscured features that are implemented sporadically (not all API functions support them):
So, the switch to 3.0 will be straight forward for most users. The Typescript client, however, will affect all users as we're also rewriting the transport layer. The options you provide during client instantiation will be different. Legacy apps will also be affected by the use of modern ES6 and Having both 3.x and TS clients existing in parallel affords the user some time to modernize their apps. With 3.x released, we can also spend more time to build the TS client (and do it right) as the API updates will be done completely via a client generator.
It's possible, but it's not straight forward. This feature is also a performance issue since it loops through the list of params to reconstruct the querystring params object for every API call. We will also run into a complication with the |
This can be achieved by shipping older versions from a 2.x branch. I think it's a small but important nuance whether we use two separate repos or not. |
Our release pipeline always uses what's in the main branch. So, having 2 different release configs in the same repo is not possible unfortunately. Moreover, since the TS client is a complete rewrite, it will have its own list of issues, and having these issues in a separate repo is easier to manage. |
I've just uploaded the JS Client 3.0 Preview into this feature branch. It was completely generated from the OpenSearch API Spec, and it is fully functional (The types are still from 2.x. Working on generating the types from the spec next). It has all the changes and improvements mentioned in the OP. I also refactored a lot of the code: |
Another breaking change we should consider for 3.0 is ending support for older Node.js versions. The support of old Node.js (We current support Node 10 and up) is preventing us from upgrading dependency packages with tsd as the newest example, and it also poses a security risk. (Node 16 stopped receiving security updates on Sep 11, 2023) |
I'd like support for Node.js 0.4. 🤡 |
Just had a discussion with @timursaurus on the TS client:
@timursaurus is going to research the current transport layer to see what business logic should be carried over to the new client. He'll work on a POC and publish his findings for discussion in a different issue in this repo. I'll modify the API generator (which is still WIP) for JS 3.0 to build the API layer for the TS client. |
A goal for 3.X should be allowing in-flight requests to be cancelled using an AbortSignal rather than an abort method on the returned Promise. |
Updates on type generation:
Similar to the main code, in 3.0, types are also divided into more readable files instead of a handful of giant files. Remaining work regarding type generation:
|
Happy to announce that we're ready for const { Client } = require('@opensearch-project/opensearch'); you will need to use const { Client } = require('./index'); // be mindful of where your test file is relatively to the `index.js` file in the root of the branch to load the source package. The On top of the issues mentioned in the previous update, the typing for the API have been completely overhauled. There's now only 1 type definition for Client. This will be a huge quality-of-life (QoL) change from 2.X where we had 3 different type definitions for Client. The generated Client type also now correctly reflects whether Would love to have some input from the dashboard team as well especially after the beta version is released. |
@nhtruong Excited to try it! Can we do 3.0.0.beta1 or will the next beta update that tag? I am sure it won't be the first and last beta ;) |
3.0.0-beta.1 has been released. Looking forward to hearing back from you! |
I tried the version in https://github.com/dblock/opensearch-node-client-demo and it worked well for the basic search scenario. The issues I'm seeing.
|
Just tested this myself on my IDE (JetBrains WebStorm). Typing works on both 2.X and 3x, and on both
I've taken some screenshots to compare between 3.x and 2.x (both in Typescript) for this code snippet: import { Client as Client3 } from 'opensearch_3'; // 3.0.0-beta.1
import { Client as Client2 } from 'opensearch_2'; // 2.11.0
// Uncomment 2 lines below for CommonJS
// const Client3 = require('opensearch_3').Client; // 3.0.0-beta.1
// const Client2 = require('opensearch_2').Client; // 2.11.0
const clientOptions = { node: 'http://localhost:9200' } // simplified
const client3 = new Client3(clientOptions);
const client2 = new Client2(clientOptions);
const start = async function() {
const nodes_info_3 = (await client3.nodes.info({ metric: [] })).body;
const search_3 = (await client3.search({})).body;
const cat_indices_3 = (await client3.cat.indices({ format: 'json' })).body;
console.log(nodes_info_3);
console.log(search_3);
console.log(cat_indices_3);
const nodes_info_2 = (await client2.nodes.info({ metric: [], })).body;
const search_2 = (await client2.search({})).body;
const cat_indices_2 = (await client2.cat.indices({ format: 'json' })).body;
console.log(nodes_info_2);
console.log(search_2);
console.log(cat_indices_2);
}
start(); Both 3.x and 2.x have good coverage for API function params: However, types in 3.x has much higher resolutions. When you use but not in 2.x The response bodies in 3.x are also of much higher resolution and more accurate. Response body for Notice how in 3.x above you get all the properties of a search response body, while in 2.x below, it's just a generic Object. Each cat endpoint (when while 3.x correct determines that it's it's an array: and the data type of the items are of a very high resolution: |
I am with you - VScode is having a separate issue that hasn't changed with 3.x, but we should fix it anyway. #839 |
@dblock The use of generic prevents VScode from properly determine the actual body of a response. While JetBrains can properly determine the response types, its While I'm at it, I'm also going to restructure the request/response type definitions so that they are importable, and have more meaningful names instead of |
Releasing beta.2 where the response types are no longer generic parameters for However, in CommonJS (i.e. using Goals for
|
@dblock found the issue in the generator. The fix will be in |
releasing beta.3 where all Request and Response objects have unique names, and the fix for this. It resolves the ambiguity for WebStorm when determining which Request and Response objects to used for autocomplete in CommonJS files. @dblock, let me know if you find any issues with VSCode in beta.3 There's one last QoL change that I'm working on in import type { Search_RequestBody } from 'opensearch_3/types/functions/search';
import type { Nodes_Info_Request } from 'opensearch_3/types/functions/nodes.info';
import type { CreatePit_Request} from 'opensearch_3/types/functions/create_pit';
...
const createPit_params: CreatePit_Request = { allow_partial_pit_creation: true, index: [movies] }
const createPit = await client.createPit(createPit_params)
console.log(createPit.body.pit_id);
const nodes_info_params: Nodes_Info_Request = { metric: ['jvm'] }
let nodes_info = await client.nodes.info(nodes_info_params);
console.log(nodes_info.body.nodes[0].jvm.mem.direct_max_in_bytes);
const search_body: Search_RequestBody = { query: { match: { director: 'miller' } } };
const search = await client.search({ index: movies, body: search_body });
console.log(search.body.hits.hits[0]._source); In import type * as API from 'opensearch_3/types/api';
...
const createPit_params: API.CreatePit_Request = { allow_partial_pit_creation: true, index: [movies] }
const createPit = await client.createPit(createPit_params)
console.log(createPit.body.pit_id);
const nodes_info_params: API.Nodes_Info_Request = { metric: ['jvm'] }
let nodes_info = await client.nodes.info(nodes_info_params);
console.log(nodes_info.body.nodes[0].jvm.mem.direct_max_in_bytes);
const search_body: API.Search_RequestBody = { query: { match: { director: 'miller' } } };
const search = await client.search({ index: movies, body: search_body });
console.log(search.body.hits.hits[0]._source); I also found another issue in the spec while testing await client.createPit({ index: 'movies' }) // This will cause a typescript error
await client.createPit({ index: ['movies'] }) // This is the only accepted format @Xtansia is this also a problem for the JAVA client? |
Works well with |
Releasing beta.4:
import { Client, type API } from '@opensearch-project/opensearch'
const createPitParams: API.CreatePit_Request = { allow_partial_pit_creation: true, index: [movies] }
const nodesInfoParams: API.Nodes_Info_Request = { metric: ['jvm'] }
const searchBody: API.Search_RequestBody = { query: { match: { director: 'miller' } } }
|
Hello and sorry to jump in with a kind of dumb question/point... I'm wrapping this client in a package for NestJS applications and I'm exporting all your types for convenience. I couldn't find any way to type search responses: const s = (await this.osClient.search(q)).body.hits.hits.map(
(el) => el._source,
);
Are you planning to use generics in the new codebase in order to enable such use-case? Or am I already missing something in the current implementation? |
@andreafspeziale Unfortunately what see now was generated directly from the OpenSearch API Spec. As of right now, the spec doesn't have any directive to specify that a certain property is a generic parameter. So instead of const s = (await this.osClient.search<MyDocument>(q)).body.hits.hits.map(
(el) => el._source,
); Now you'll do const s = (await this.osClient.search<MyDocument>(q)).body.hits.hits.map(
(el) => el._source as MyDocument,
); For clarification, the generics we got rid of in beta.2 was the body property of the ApiResponse interface. Instead, we define every response as an extension of ApiResponse to replace the body with the response body of the endpoint like in search for example. |
There aren't many of those, maybe we can keep a workaround in the generator? |
We could but It won't be a quick fix unfortunately. The generic is actually nested in several layers: If we're to do this, I'd say we should put this info in the spec instead of hardcoding it in the generator because of how complex the Generic chain is already to handle, and adding some logic to recognize the edge-case as a workaround will make things even more complex. This brings up another question: Should we hold up 3.0.0 to work on this feature (It could take awhile), or save this for 3.1.0 because the user actually have a workaround to tell TS that |
@andreafspeziale Do you think the work around described here ( |
@nhtruong I would save it for later. P.S: I moved to Those are all the options I can see, it seems I can't resolve
|
Even though they exist in the source code: Not sure how that happened. Edit: NVM found the issue. 🤦 |
Release beta.5:
|
@andreafspeziale how did you use generic for
So to actually use the client type declaration with TDocument generic in 2.11, it's rather convoluted: import { Client } from '@opensearch-project/opensearch';
import { type Client as NewClient } from '@opensearch-project/opensearch/api/new';
const client = (new Client({...})) as unknown as NewClient; // have to coerce it to unknown first
const search = (await client.search<MyDocument>({...})).body;
const doc = search.hits.hits[0]._source; // Then TS will see doc as MyDocument |
@nhtruong Sorry for the late response 😞 I'm not using 2.x in my projects, I'm already on
(The second is leveraging the first) Drop a ⭐ if you found them interesting! Have been developed with ❤️ in my free time |
@andreafspeziale Wow those are great projects. And thanks for helping us test the beta. |
No problem @nhtruong! Thx for your hard work people! |
As a part of our effort to generate all clients from the OpenSearch API Spec, and modernize the Node.js client, we are planning some major updates for this repo.
OpenSearch JS Version 3.0
The next major version of this client is going to be fully generated from the API Spec. The vast majority of the current client is hand-written with a lot of inconsistency in the implementations of the less known and less used features:
Said features are going away in 3.0 to promote a cleaner and more efficient code base:
method
param to the function. That means, usages of this feature will result in error from the server as themethod
param will be treated as a query string parameter.Version 3.0 will also introduce other improvements:
.js
file.OpenSearch Typescript
Once OpenSearch JS 3.0 is released, we will start rewriting this client in Typescript in a different repo. This rewrite will carry over all improvements introduced in OpenSearch JS 3.0, along with some other changes to modernize and streamline the client:
async/await
syntax.The Typescript client will be a completely different client, and we're aiming to do it properly with ample research. OpenSearch JS 3.x will be constantly be updated when OpenSearch Typescript is under construction, and it will continue receive API updates long after OpenSearch Typescript is released to give the user time to switch over.
If you have any concern or suggestion, feel free to comment below.
The text was updated successfully, but these errors were encountered: