aws-beam / aws-codegen

Code generator for AWS clients in Erlang and Elixir.
Other
53 stars 49 forks source link

Possibility of Structured input #65

Closed ymtszw closed 6 months ago

ymtszw commented 3 years ago

The approach of official-schema-originated code generation is great.

Is there a plan to provide further support for structured interfaces, particularly structured inputs? Currently inputs for generated API functions are mere maps, (example: https://github.com/aws-beam/aws-elixir/blob/master/lib/aws/generated/acm.ex#L55)

  def add_tags_to_certificate(%Client{} = client, input, options \\ []) do
    Request.request_post(client, metadata(), "AddTagsToCertificate", input, options)
  end

but schema JSON provides inputs and their shape definitions. (https://github.com/aws/aws-sdk-go/blob/main/models/apis/acm/2015-12-08/api-2.json#L16-L32)

    "AddTagsToCertificate":{
      "name":"AddTagsToCertificate",
      "http":{
        "method":"POST",
        "requestUri":"/"
      },
      "input":{"shape":"AddTagsToCertificateRequest"},
      "errors":[
        {"shape":"ResourceNotFoundException"},
        {"shape":"InvalidArnException"},
        {"shape":"InvalidTagException"},
        {"shape":"TooManyTagsException"},
        {"shape":"TagPolicyException"},
        {"shape":"InvalidParameterException"},
        {"shape":"ThrottlingException"}
      ]
    },

(https://github.com/aws/aws-sdk-go/blob/main/models/apis/acm/2015-12-08/api-2.json#L240-L250)

    "AddTagsToCertificateRequest":{
      "type":"structure",
      "required":[
        "CertificateArn",
        "Tags"
      ],
      "members":{
        "CertificateArn":{"shape":"Arn"},
        "Tags":{"shape":"TagList"}
      }
    },

My naive idea is to generate structs for shapse of "type": "structure" inputs (for aws-elixir), and accept them as input arguments of API functions.

philss commented 3 years ago

@ymtszw The idea is good! The only problem is that it will probably increase the compilation time a lot, because it would require the definition of a bunch of new modules.

One possibility would be to add typespecs and define which key is optional and which one is required. In conjunction with better documentation explaining each option can help developers. Another thing is to reduce the number of arguments needed. An approach similar to https://github.com/aws-beam/aws-codegen/pull/64 seems to solve this problem. WDYT?

ymtszw commented 3 years ago

One possibility would be to add typespecs and define which key is optional and which one is required. In conjunction with better documentation explaining each option can help developers.

I think it is totally acceptable. The main motivation was two-fold: 1) provide pseudo-API-doc in the code, eliminating need for looking up external docs, and 2) enforce API conformance at compile-time (by utilizing struct, for instance.)

At least 1) can be achieved with typespecs and document generation well enough, without compromising compile speed. And it potentially helps 2) via dialyzer, if we are lucky with it.

andyleclair commented 2 years ago

Hi! So I had some time, and I tried to write up the code that it would take to generate all of the structs (and types) and you are correct, it takes forever to compile, like, upwards of 20 minutes on my Ryzen 9.

I've opened a PR so you can look at the code: https://github.com/aws-beam/aws-codegen/pull/87

onno-vos-dev commented 6 months ago

Now that we've migrated to the V2 of SDK Go and that is out of the way, my intention is to start looking into this issue over the coming weeks 👌

I really like the idea and thanks again @andyleclair for the initial work. Hoping to base it upon that and if so, you'll get the Co-Author tagging in the commit for sure ❤️

onno-vos-dev commented 6 months ago

As you mentioned in PR #87 it's extremely slow to compile with structs everywhere. I've now opened a draft PR #108 to add specs which should hopefully be a good starting point without spending an eternity at compile time 👍

@andyleclair @ymtszw feel free to comment on that PR (or here) if that'd be a decent way forward 👍

onno-vos-dev commented 6 months ago

Closing. Best we can do (for now) is: https://github.com/aws-beam/aws-codegen/pull/109

andyleclair commented 6 months ago

@onno-vos-dev have you considered breaking each service out into its' own repo? That would allow users to not have to pull in all of AWS and I think having the structs wouldn't affect compile time (as much, since there's just less to do)