amazonka icon indicating copy to clipboard operation
amazonka copied to clipboard

Tracking issue: spurious `Maybe`s in request/response bodies

Open endgame opened this issue 3 years ago • 2 comments
trafficstars

At some point, the generator was changed to emit Maybe values for all optional fields, even for types that had a sensible "empty" value (e.g., maps and lists). Among other benefits, this made it possible to detect empty lists and maps in DynamoDB AttributeValues (before #724 introduced an AST and solved things properly).

There's a big QoL downside to this, particularly for responses: it makes a lot of request/response fields look optional, and when handling responses you're forced to write partial code or handle errors that should never appear.

I've tried raising issues for these in botocore, but AWS service teams have a habit of not being very thorough when marking things as required in their service definitions (and changing this risks backwards-compat problems now).

Fixing these is not that difficult; it's just an additional requiredFields entry in configs/$SERVICE.json and would make a good first PR. If you make an issue or PR about one of these, please:

  1. Mention this issue;
  2. Provide proof that the field is always present (in the form of AWS documentation or a link to code in the SDK); and
  3. Tag it service config
  4. Regenerate the relevant service by running scripts/generate --commit $serviceabbrev (e.g., dynamodb)

If you have questions about a specific field and want to discuss whether or not to make a PR, please create an issue for it, mention this issue in its description, and give it the service config label.

endgame avatar Dec 16 '21 00:12 endgame

I am a bit unsure about the Value field on the Parameter type of AWS Systems Manager (formerly known as SSM).

It is required according to docs on https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ssm-parameter.html

These docs don't mention anything about optionality, but they do mention the field and supply it in their examples:

  • Systems Manager API reference for get_parameter method https://docs.aws.amazon.com/systems-manager/latest/APIReference/API_GetParameter.html
  • Ruby Parameter docs https://docs.aws.amazon.com/sdk-for-ruby/v3/api/Aws/SSM/Types/Parameter.html
  • Python get_parameter docs https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/ssm.html#SSM.Client.get_parameter

In the Go bindings, it is a pointer, which suggests that it is optional. But it seems like everything is a pointer. https://docs.aws.amazon.com/sdk-for-go/api/service/ssm/#Parameter

This piece of Parameter documentation claims it is not required: https://docs.aws.amazon.com/systems-manager/latest/APIReference/API_Parameter.html

So what to do? Don't know whether I should make a PR.

ysangkok avatar Dec 20 '21 21:12 ysangkok

@ysangkok I moved your question to a new issue, otherwise this issue will get confusing over time. Actually, I think I'm going to lock it to prevent the from happening.

endgame avatar Dec 20 '21 21:12 endgame