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

[PROPOSAL] Remove http method override, camel-cased params, and snake-cased function aliases. #676

Open
nhtruong opened this issue Dec 18, 2023 · 1 comment

Comments

@nhtruong
Copy link
Collaborator

The 3 features mentioned in the title are unique to the JS client:

HTTP Method override:

Most API functions in this client allows the user to pass method as a parameter to override the HTTP method specified for the endpoint. For example, you can technically force the client to use HTTP POST on _cat/indices endpoint. Of course this will result in an error from OpenSearch.

I'm not sure what the purpose of this feature was, but it makes very little sense to have it now. Note that non of the other clients support this. If you're aware of why this feature was added, please chime in :)

Camel-cased API parameters

While OpenSearch only accepts snake-cased parameters (e.g. scroll_id), many (but not all, since this feature is not implemented consistently) JS API functions also accept the camel-case equivalence (.e.g scrollId). These functions then uses a map to translate the camel-cased params to OpenSearch snake-cased params.

While JS naming convention uses camel-case, I find this feature unnecessary and may even be confusing since params like scrollId never appear in any of our API docs, and using params like that in a curl command will result in errors. This features also create overhead for the client, increases the complexity of each API implementation (and the up-coming API generator), and makes the code base harder to read.

Snake-cased API function aliases

Similarly, the client also supports 2 naming conventions for the API function names. For example, while the cat namespace explicitly defines pendingTasks, it also supports pending_tasks as an alias as well.

The use of camel-case to name function JS functions in this case make sense. The snake-cased aliases, however, is unnecessary in my opinion.


So, I propose that we remove these features from the JS client to reduce the code base's and the API Generator's complexity, and put it in line with other clients. Do note that removing these features, especially the last 2, will result in breaking changes for some users. We can implement this change along with the API Generator I'm working on (That is the newly generated API functions wont support these features).

@dblock
Copy link
Member

dblock commented Dec 19, 2023

All 3 make sense to me with a major version bump where the entire client is generated from spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants