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

[FEATURE] Generate API code from OpenSearch API Spec #284

Open
VachaShah opened this issue Apr 6, 2023 · 5 comments
Open

[FEATURE] Generate API code from OpenSearch API Spec #284

VachaShah opened this issue Apr 6, 2023 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@VachaShah
Copy link
Collaborator

Is your feature request related to a problem?

Related to opensearch-project/opensearch-clients#19

What solution would you like?

Re-use the generator that generates API Code that maintains backwards compatibility with the current client APIs and provides a way to add new APIs.

What alternatives have you considered?

Using OpenAPI generators instead of re-using the existing generator but it makes it difficult to maintain backwards compatibility.

Do you have any additional context?

Add any other context or screenshots about the feature request here.

@tannerjones4075
Copy link
Contributor

I would like to help with this issue. Is it still relevant?

@VachaShah
Copy link
Collaborator Author

Hi @tannerjones4075, yes absolutely! The first step would be use to the API specs from https://github.com/opensearch-project/opensearch-api-specification/blob/main/OpenSearch.openapi.json and modify the existing non-functional generator in the go client to create code similar to the existing client. The general idea is to replace all the code (module by module or divided in whatever way works) in this repo for the APIs with code generated from the API specs without breaking the users.

@Jakob3xD
Copy link
Collaborator

I am currently trying to generate sturcts out of the component schemas and I don´t think I can continue at this point.
There are lots of object properties defined which no longer exist in the API and are referencing to types that can not be handled in opensearch.

component/schemas/_common:ByteSize is of type oneOf pointing to type number or string.
Inside golang it is not possible for a field to be both. We would need to set the field to json.RawMessage which is just a wrapper for any, forcing the user to find out the type of the field on his own.
_common:ByteSize is referenced quite some time but when going though the the OpenAPI spec and our current golang code, I noticed that many fields that reference _common:BysteSize are no longer returned by the API as they got replaced by the same property suffixed with _in_bytes pointing to type number.
As we validate all our golang structs against the API and check for missing fields, I assume that those fields must be very old as they never existed in any Opensearch 1.x o 2.x release.

For example:

https://github.com/opensearch-project/opensearch-api-specification/blob/9c15bbf1522b1604e2e36f16b53163413f5fe833/spec/schemas/_common.yaml#L982-L986

Should I open an issue for that in the api spec repository? @dblock
Similar issue for component/schemas/_common:Percentage. It is also oneOf for number or string.
This is used for the cluster health endpoint. Which returns the value as number for yaml and json.

Another issue that we might be able to handle is the use of oneOf pointing to a reference or an array of the same reference. Meaning the field be a direct object/type or and array of that object/type.
https://github.com/opensearch-project/opensearch-api-specification/blob/9c15bbf1522b1604e2e36f16b53163413f5fe833/spec/schemas/_common.yaml#L342-L346
In most cases those schemas are referenced in the parameters where we can assume the array type to work.

Some more information:
Nearly all anyOf are used to mark values as nullable which should be removed once the issue: opensearch-project/opensearch-api-specification#366 is handled.

@dblock
Copy link
Member

dblock commented Jul 16, 2024

First, I'm really glad to see you look into this @Jakob3xD!

I am currently trying to generate sturcts out of the component schemas and I don´t think I can continue at this point. There are lots of object properties defined which no longer exist in the API and are referencing to types that can not be handled in opensearch.

If you see bugs/issues in the spec you can/should fix them or open issues, absolutely yes, please. We inherited a lot from Elastic.

component/schemas/_common:ByteSize is of type oneOf pointing to type number or string. Inside golang it is not possible for a field to be both. We would need to set the field to json.RawMessage which is just a wrapper for any, forcing the user to find out the type of the field on his own.

Either the spec is incorrect or the spec is correct and we should handle it. Feel free to open an issue if you're unsure, as always.

_common:ByteSize is referenced quite some time but when going though the the OpenAPI spec and our current golang code, I noticed that many fields that reference _common:BysteSize are no longer returned by the API as they got replaced by the same property suffixed with _in_bytes pointing to type number. As we validate all our golang structs against the API and check for missing fields, I assume that those fields must be very old as they never existed in any Opensearch 1.x o 2.x release.

For example:

https://github.com/opensearch-project/opensearch-api-specification/blob/9c15bbf1522b1604e2e36f16b53163413f5fe833/spec/schemas/_common.yaml#L982-L986

Should I open an issue for that in the api spec repository?

You're definitely right. I opened opensearch-project/opensearch-api-specification#423, but in general please don't hesitate to open issues even if you're unsure.

Another issue that we might be able to handle is the use of oneOf pointing to a reference or an array of the same reference. Meaning the field be a direct object/type or and array of that object/type. https://github.com/opensearch-project/opensearch-api-specification/blob/9c15bbf1522b1604e2e36f16b53163413f5fe833/spec/schemas/_common.yaml#L342-L346 In most cases those schemas are referenced in the parameters where we can assume the array type to work.

One way you can handle this is to roll out custom types without having to parse it or always narrow it down to a primitive type. If something is of FooBarType you could make an actual GoLang struct called FooBar, no?

Some more information: Nearly all anyOf are used to mark values as nullable which should be removed once the issue: opensearch-project/opensearch-api-specification#366 is handled.

Right. LMK which specific issues are blocking you and I will look into them.

@Jakob3xD
Copy link
Collaborator

To give an update, to better understand the issue and my last comment.

ByteSize

I remove the ByteSize type in the openapi spec and replaced it with a string type component and an integer type component to remote the anyOf reference.
opensearch-project/opensearch-api-specification#552
There is still an open discussion how we can improve the naming.

"Old" fields

There are lots of object properties defined which no longer exist in the API

Looking deeper I notices this statement is false. The fields still exists but are "hidden" behind the human parameter.
I also tried to adjust the referenced in the MR named above.

Percentag

I haven't looked into the Percentage oneOf problem. Maybe this can also be split up into two components with a fixed type.

Null

The nullable field got removed in favor of the null type. opensearch-project/opensearch-api-specification#436
If a component has type null in it, it needs to be a pointer.

Issues (i currently know of)

anyOf/oneOf

Components using anyOf or oneOf need to be handled. For components that use them to not define two components, like it was with the BytesSize component, they should be split into components with a single type.

Components using anyOf or oneOf to reference the component directly or as an array need to be handled.
If the Component is only used as Parameter we could just assume the array type. A "simpler" more fast forward solution wold be to use type any, in combination with an open issue to improve this.

omitempty

How do we know where to set the omitempty flag.
Haven't tough about this much but maybe the required openapi field can help with this.
This is very important when it comes to request structs. For example you want to update a single filed, then other fields most likely needs to be omitted so they don't overwrite things with empty strings/arrays.





I am currently not working on this, as IMO the openapi spec needs to be improved first to work better with golang.
However you can also prove me wrong that the current openapi spec works fine with golang or you have good ideas to handle the issue I ran into :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants