Provide high-level response types that don't contain `Option<T>` for required fields

[Veetaha]

trafficstars

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 the problem you're trying to solve. What are you trying to do, and why is it hard? 99% percent of response types in AWS API declarations contain Option<T> fields. For example the Instance struct that represents an EC2 instance in describe_instances() response is the following (I've removed comments, and irrelevant noise):

pub struct Instance {
    pub ami_launch_index: Option<i32>,
    pub image_id: Option<String>,
    pub instance_id: Option<String>,
    pub instance_type: Option<InstanceType>,
    pub kernel_id: Option<String>,
    pub key_name: Option<String>,
    pub launch_time: Option<DateTime>,
    pub monitoring: Option<Monitoring>,
    pub placement: Option<Placement>,
    pub platform: Option<PlatformValues>,
    pub private_dns_name: Option<String>,
    pub private_ip_address: Option<String>,
    pub product_codes: Option<Vec<ProductCode>>,
    pub public_dns_name: Option<String>,
    // ...
}

Basically all of the fields in this type are Option.

Does it mean that AWS API can return e.g. an empty object ({}) as a valid response? Yes it does.

Does it mean that AWS API should return e.g. an empty object ({}) as a valid response? No it doesn't.

It creates a lot of boilerplate code when using the SDK libraries that handles all of these option types. And that boilerplate is not always the same. Some people use unwrap()/expect() (please don't this), some people use unwrap_or_default()/filter_map() to coerse to default / filter None values. We do the latter + create a warn!() log event to notify us about the fact that None was experienced where it wasn't expected, but we just ignored it.

All this boilerplate is on SDK user's shoulders...

Also, take a look at Rust SDK code examples in the official AWS documentation. It is full of unwrap()s! That's just ridiculous! The official documentation must propagate the idiomatic usage of the SDK, not the way that no-one will ever use it. Having unwrap()s here and there will lead to panics, and we don't want our application to panic if AWS API return None somewhere!

Are you currently working around this issue? We currently don't use this SDK, but we use rusoto crates that suffer from all the same problem. The workaround is to handle Option<T> fields manually in code. We have an entire wrapper crate over all rusoto APIs where we try to get rid of Option<T> types and filter response elements that contain None fields with warnings closer to the communication layer. However, that costs us a lot of time and money to support. And it costs everyone who uses the Rust SDK time and money to implement this logic every time we use Rust SDK unless there is a centralized solution for this provided by AWS.

Additional context I understand all these types come from API schema declarations that declare all fields as optional, so it is most likely a more general problem that other SDKs might suffer from. However, since we care about Rust, we would like to have some convenience around handling all of these Option<T> fields here specifically. So even if the core schema changes to make them required this issue is here to support that new schema then.

Sorry for the emotional speech. I think people generally agree with the pain I am describing here, I hope it's not too emotional

[rcoh]

Thanks for the feedback on this and thanks to everyone who's quickly responded with upvotes. The impact this has on our prioritization can't be overstated.

Some discussion is here: https://github.com/awslabs/aws-sdk-rust/discussions/163. We hope to make many of these fields required prior to GA.

For some context as to why this is taking so long on our side: If we get it wrong and a field that we think is required is actually optional and we offer it as a required field, this would almost certainly result in runtime panics or deserialization failures with the only recourse being that customers would need to upgrade their SDK. (This happens sometimes with Rusoto due to modeling issues of the AWS APIs).

[jdisanti]

Tracking this in #536.

[github-actions[bot]]

⚠️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.