packngo icon indicating copy to clipboard operation
packngo copied to clipboard

Take a serious look at generating this client from Swagger

Open displague opened this issue 3 years ago • 7 comments

The Equinix Metal API is available via Swagger (OpenAPIv2) at https://api.equinix.com/metal/v1/api-docs/.

Given the availability of https://goswagger.io/ and https://github.com/OpenAPITools/openapi-generator-cli we should be able to build a template (docker run --rm -v $PWD:/local openapitools/openapi-generator-cli:v5.0.0-beta2 meta) or set up options that will allow for a partial client to be built from the swagger spec.

  • What translations may be needed against the Swagger before processing the document? (Convert to OpenAPI3, either upstream or downstream?)
  • What upstream X- Swagger parameters might be needed for this to be successful? (x-nullable: true?)
  • How will we handle pagination, since pagination is a OpenAPIv3 defined property not included in the OpenAPIv2 spec?
  • What actions are more involved than single API calls?
  • Can we and how do we git a generated client to fit the existing pattern?
  • If a generated client would differ greatly, how would we (where would we) package this new client up?
  • If we are left stubbing over functions or parameters that the generated client does not produce, can we create custom generators to simplify or avoid that work?

... if a program writes your code for you, the code will be more correct and reliable than if you write it yourself by hand. If the generator is improved, for example, to produce better code, everyone benefits; by contrast, improvements to one hand-written program do not improve others. -- Brian Kernighan

displague avatar Nov 18 '20 19:11 displague

I've used the openapitools/openapi-generator-cli generator to produce the (likely unusable) https://github.com/displague/packet-rs. Each commit and commit description reports the Swagger changes that were needed to fix validation and generate the client.

displague avatar Nov 18 '20 19:11 displague

I tried to generate API client with swagger based on the api-docs ~3 years ago. The problem there IIRC was that the API docs were not consistent with the true state of the API, making the generated code pretty much unusable.

(this is not a useful piece of info) I also remember feeling it turned up much harder to do than it sounds (to simply generate api client based on json description).

t0mk avatar Nov 21 '20 12:11 t0mk

I believe if the swagger/openapi is well done then I'm convinced that the generated SDK should be good. Of course one cannot expect it to be perfect without human intervention. Also it is a good thing to keep SDKs standardized.

jasmingacic avatar Nov 21 '20 15:11 jasmingacic

Looking at #228 I've just tried to use swagger on api-docs.json. These were the issues:

  • host in body should match '^[^{}/ :\]+(?::\d+)?$'
    (doesn't like forward slashed in "host": "api.equinix.com/metal/v1")
  • items in definitions.BgpNeighborData.properties.peer_ips is required (missing items description dict)
  • duplicate parameter name "id" for "path" in operation "findConnectionPortEvents"
  • params in path "/connections/{id}/ports/{id}/events" must be unique: "{id}" conflicts with "{id}"
  • "user_id" is present in required but not defined as property in definition "SupportRequestInput"

t0mk avatar Dec 14 '20 10:12 t0mk

I tried to generate and run a client with go-swagger. This is my test repo, I put some notes to README: https://github.com/t0mk/gometal

t0mk avatar Dec 14 '20 14:12 t0mk

A little update on the progress here:

  • the client generation process is scripted: https://github.com/t0mk/gometal/blob/main/gen.sh
  • the upstream API spec has less of the validation/accuracy problems encountered above
  • patches are applied (from gometal) for the known problems
  • several issues have been opened to further explore the potential use of this client

displague avatar Jan 25 '21 16:01 displague

This work has the potential to be a #213 implementation.

displague avatar Jan 25 '21 16:01 displague