tannerjones4075
commented
12 months ago I would like to help with this issue. Is it still relevant?
Open VachaShah opened 1 year ago
tannerjones4075
commented
12 months ago I would like to help with this issue. Is it still relevant?
VachaShah
commented
12 months ago 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
commented
2 months ago 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:
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: https://github.com/opensearch-project/opensearch-api-specification/issues/366 is handled.
dblock
commented
2 months ago 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:ByteSizeis of typeoneOfpointing to typenumberorstring. Inside golang it is not possible for a field to be both. We would need to set the field tojson.RawMessagewhich is just a wrapper forany, 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:
Should I open an issue for that in the api spec repository?
You're definitely right. I opened https://github.com/opensearch-project/opensearch-api-specification/issues/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
oneOfpointing 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
anyOfare 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
commented
2 weeks ago To give an update, to better understand the issue and my last comment.
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. https://github.com/opensearch-project/opensearch-api-specification/pull/552 There is still an open discussion how we can improve the naming.
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.
I haven't looked into the Percentage oneOf problem. Maybe this can also be split up into two components with a fixed type.
The nullable field got removed in favor of the null type. https://github.com/opensearch-project/opensearch-api-specification/pull/436 If a component has type null in it, it needs to be a pointer.
anyOf/oneOfComponents 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.
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
Is your feature request related to a problem?
Related to https://github.com/opensearch-project/opensearch-clients/issues/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.