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

[request]: sdk model structs should not have Optional type if the field is marked as required in smithy. #267

Closed
asanthan-amazon opened this issue Oct 27, 2021 · 5 comments
Labels
breaking-change This issue requires a breaking change to remediate. feature-request A feature should be added or improved.

Comments

@asanthan-amazon
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue, please leave a comment

Tell us about your request
Currently, all the fields in generated types are marked as Optional. But, the sdk should generate types where fields should not be marked as Optional if it is marked as @required in Smithy.

Tell us about the problem you're trying to solve. What are you trying to do, and why is it hard?
The generated sdk model types should not have Optional type if the field is marked as required in smithy.

Are you currently working around this issue?
Because, it is marked as Optional, there is no compile-time validation to say if the entire type is fully constructed or not.
As a work around, we create another wrapper type which does not use Optional for required fields.

Additional context
Anything else we should know?

Attachments
If you think you might have additional information that you'd like to include via an attachment, please do - we'll take a look. (Remember to remove any personally-identifiable information.)

@asanthan-amazon asanthan-amazon added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 27, 2021
@Velfi
Copy link
Contributor

Velfi commented Oct 27, 2021

Thanks for submitting this issue!

The reason for this is that in smithy it's not considered a breaking change to add or remove the required trait. However, we do recognize that the user experience could be improved in this area and we're open to solutions.

@Velfi Velfi added the breaking-change This issue requires a breaking change to remediate. label Oct 27, 2021
@rcoh
Copy link
Contributor

rcoh commented Nov 1, 2021

see also #163

@rcoh rcoh removed the needs-triage This issue or PR still needs to be triaged. label Nov 1, 2021
@asanthan-amazon
Copy link
Author

I think if the service team removes required trait, that is backward incompatible change from my perspective. In which case, a new version of SDK needs to be generated that customer can use.

In that way, customer will come to know explicitly that an optional field now becomes mandatory or vice-versa.

@jdisanti
Copy link
Contributor

Tracking this in #536.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This issue requires a breaking change to remediate. feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

4 participants