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

Auto Generated API - 3.0.0 #864

Merged
merged 2 commits into from
Sep 10, 2024
Merged

Conversation

nhtruong
Copy link
Collaborator

@nhtruong nhtruong commented Sep 4, 2024

Changed

  • All API functions are now generated from the OpenSearch API specification.
  • API request and response types are now generated from the OpenSearch API specification.
  • Overhauled API codebase and break it into smaller, more manageable files for better readability and maintainability.

Removed

  • BREAKING Removed support for API param aliases. That is, the API functions now only accept params with the exact names specified in the OpenSearch API specification.
  • BREAKING Removed support for snake_cased API function names. All API functions are now camelCased only to conform to JavaScript naming conventions.
  • BREAKING Removed support for Node.js 10 through 16. The minimum supported Node.js version is now 18.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock
Copy link
Member

dblock commented Sep 4, 2024

Let's branch 2.x before we merge this in order to have patch releases?

@nhtruong nhtruong force-pushed the api_3.0 branch 2 times, most recently from eec46f0 to d43db2b Compare September 4, 2024 23:29
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looking good.

I don't know how much we care, but a very large amount of files are changed for a 1-liner in the license header. Adding an extra * would reduce this PR by half I imagine.

Screenshot 2024-09-05 at 7 04 01 AM

Another though on the camelcase/snake case. Having to update all code from one method to the other is going to be a barrier for users even though it's mostly a trivial change. Would it make sense to consider also generating wrapper methods with the old names and marking them deprecated for another major version iteration? Or to generate a compatibility layer that can be optionally included?

CHANGELOG.md Outdated Show resolved Hide resolved
@nhtruong nhtruong force-pushed the api_3.0 branch 2 times, most recently from c4f510b to 447b094 Compare September 6, 2024 23:59
@nhtruong nhtruong marked this pull request as ready for review September 6, 2024 23:59
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I tried to go through all the changes, made some suggestions in text.

I think after this is merged we should get another beta off main bofore releasing a final version. The final should be basically like the previous beta.

UPGRADING.md Outdated Show resolved Hide resolved
UPGRADING.md Outdated Show resolved Hide resolved
UPGRADING.md Outdated Show resolved Hide resolved
UPGRADING.md Show resolved Hide resolved
lib/pool/BaseConnectionPool.js Outdated Show resolved Hide resolved
@nhtruong
Copy link
Collaborator Author

nhtruong commented Sep 9, 2024

Looking good. I tried to go through all the changes, made some suggestions in text.

I think after this is merged we should get another beta off main bofore releasing a final version. The final should be basically like the previous beta.

I agree. There will be 2 follow-up PRs after this one:

  • After the Search endpoint has been fixed in the spec, which I'm working on, I'll run the generator again to apply that change among others changes since the last time the spec was pulled to generate this. We can make a last beta release after that.
  • Another PR for the generator code itself, and the supporting workflows. This can also wait till 3.1.0 release.

Signed-off-by: Theo Truong <theotr@amazon.com>
Signed-off-by: Theo Truong <theotr@amazon.com>
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Merge at will. The fact that samples don't have any changes in them is very encouraging.

@nhtruong nhtruong merged commit 0805235 into opensearch-project:main Sep 10, 2024
53 of 54 checks passed
@nhtruong nhtruong deleted the api_3.0 branch September 10, 2024 13:58
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

Successfully merging this pull request may close these issues.

2 participants