ocaml-swagger icon indicating copy to clipboard operation
ocaml-swagger copied to clipboard

Support vendor extensions

Open rdavison opened this issue 5 years ago • 5 comments

Maybe I'm not looking in the right place, but it doesn't seem like vendor extensions are supported by this library. I think the first thing we can do is to make all the vendor extensions visible to the code generator by collecting them in the swagger.atd into a private field called _vendor_extensions.

I've already started some work on this in a branch of a fork (https://github.com/andrenth/ocaml-swagger/compare/master...rdavison:watch), and I've ran into a couple of problems.

  1. It appears atdgen doesn't easily support collecting unspecified JSON object fields. So the swagger.json file has to be preprocessed. Fortunately, this can be done within atdgen by creating an adapter, such as <json adapter.ocaml="Swagger_util.Adapters.Extensions">

  2. After you have implemented the adapter, atdgen unfortunately generates ocaml code which does not compile due to at least two unrelated reasons. Thus, I had to manually fix the generated code and create a patch file (https://github.com/rdavison/ocaml-swagger/blob/92e61672e8752f110be4a609b5bc3625cf6751d2/src/patch.txt). (I'm planning to submit this as a bug to the atdgen issue tracker)

With this in place, I think it would open up the doors to making it possible to implement plugins based on vendor extensions. Any thoughts?

rdavison avatar Oct 29 '18 23:10 rdavison

Sorry for taking so long to reply. My only thought is... do you want commit rights? :)

andrenth avatar Jan 10 '19 20:01 andrenth

I'm interested in helping with this. The support for custom extensions would allow us to extract the default values for apiVersion and kind properties from the x-kubernetes-group-version-kind extension field.

@rdavison do you have any updates on this? I can try looking at the adapter you implemented.

rizo avatar Feb 04 '19 14:02 rizo

@rdavison Looks like your patch is no longer needed. The type annotation for fields is now correctly added by atdgen (fix: https://github.com/mjambon/atd/commit/8089d9e1878b3227ac5fe5d19eee48ced4abcf3a).

Update: just noticed this is already reported.

rizo avatar Feb 04 '19 15:02 rizo

@rizo, yes. I believe that the issue has been resolved by @struktured. I have done a bit of work in a fork, with partial support for vendor extensions. However, you're more than welcome to try it out. https://github.com/rdavison/ocaml-swagger

There are a few places where technically vendor extensions are supported where my branch fails to address them. You can see them here in the ATD file:

https://github.com/rdavison/ocaml-swagger/blob/master/src/swagger.atd#L215 https://github.com/rdavison/ocaml-swagger/blob/master/src/swagger.atd#L247 https://github.com/rdavison/ocaml-swagger/blob/master/src/swagger.atd#L269

The vendor extensions are collected using a custom adapter: https://github.com/rdavison/ocaml-swagger/blob/master/src/swagger_util.ml#L13-L38

Next, I'm not sure if the following is relevant to what you need it for, but anyways, I added a hook to allow another library to generate HTTP calls however it wants based on the Swagger types. I'm not completely satisfied here... still doubting whether this is the right approach, and dislike the fact that it is specific to just HTTP requests: https://github.com/rdavison/ocaml-swagger/blob/master/src/val.ml#L419-L420

Finally, I did a lot of work on a fork of Kubecaml as well to consider the vendor extensions when code-generating HTTP requests. The code walks the AST and uses information from the Kubernetes vendor extensions to codegen support for watch API calls, among other things. Warning, the code is not documented, and there may be dragons in there, but I can confirm it does work. https://github.com/rdavison/kubecaml/blob/master/gen/vext.ml

On the logistical side of things, I haven't tried to push them upstream because I would prefer to have some community feedback. Also, I lament that the code is not very well documented, but I would be interested in attempting to move this code forward with perhaps the eventual goal of upstreaming it. For example, I have not tested it with Lwt yet, which is one of the reasons it is sitting stagnant. You're more than welcome to review it and change it as needed, and I'd also be happy to work together to polish it.

rdavison avatar Feb 04 '19 18:02 rdavison

@rizo, I forgot to mention, if you would like to test those forks, the way I have been doing it has been to:

mkdir kube && cd kube
git clone [email protected]:rdavison/ocaml-swagger
git clone [email protected]:rdavison/kubecaml
dune build -p swagger,kubecaml-async
dune utop -p kubecaml-async
# alternatively, assuming you have a kubectl proxy with port open at 8001, you could run
dune exec kubecaml/examples/async/watch_example.exe -- http://localhost:8001

If you run the example code, obviously, in order to get any output, you'll have to modify some things. You may need to create/delete some namespaces.

rdavison avatar Feb 04 '19 19:02 rdavison