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

Possibility of Structured input #65

Closed
ymtszw opened this issue Mar 13, 2021 · 7 comments
Closed

Possibility of Structured input #65

ymtszw opened this issue Mar 13, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@ymtszw
Copy link

ymtszw commented Mar 13, 2021

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
Copy link
Contributor

philss commented Mar 23, 2021

@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 #64 seems to solve this problem. WDYT?

@ymtszw
Copy link
Author

ymtszw commented Mar 23, 2021

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
Copy link

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: #87

@onno-vos-dev onno-vos-dev added the enhancement New feature or request label Mar 9, 2024
@onno-vos-dev
Copy link
Member

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
Copy link
Member

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
Copy link
Member

Closing. Best we can do (for now) is: #109

@andyleclair
Copy link

@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)

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

No branches or pull requests

4 participants