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

codegen: Update Query Protocol generation to always serialized unboxed #1161

Closed
wants to merge 3 commits into from

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Mar 10, 2021

Updates the EC2Query Protocol serializer generation to always serialize unboxed member values to the querystring. This prevents issues with members values never being serialized to the request.

Fixes #1107

Updates the Query Protocol serializer generation to always serialize
unboxed member values to the querystring. This prevents issues with
members values never being serialized to the request.

Fixes aws#1107
@jasdel jasdel added the pr/needs-review This PR needs a review from a Member. label Mar 10, 2021
Copy link
Member

@JordonPhillips JordonPhillips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation wise this looks fine, but it seems potentially dangerous to be always serializing zero values. What other operations / parameters have you tested this with?

@jasdel
Copy link
Contributor Author

jasdel commented Mar 11, 2021

This is the rough part about how SDK's handle unboxed members. The API is not supposed to distinguish the difference between unset and zero value (e.g. `false). But in this case with the EC2Query protocol, the API is either treating the value as boxed, which means its modeled wrong, or the field is actually required, but not modeled as so.

In the case of #1107 not modeled as required when it should be seems to be the issue. Whereas InstanceNetworkInterfaceSpecification.DeviceIndex is modeled as an unboxed number but treated as boxed by the API.

	// The position of the network interface in the attachment order. A primary network
	// interface has a device index of 0. If you specify a network interface when
	// launching an instance, you must specify the device index.
	DeviceIndex int32

@jasdel
Copy link
Contributor Author

jasdel commented Mar 11, 2021

So going to close this PR because this solution is not valid for EC2Query since the API is treating these fields as boxed, and returns errors if zero values are sent in request when they are unexpected.

@jasdel jasdel closed this Mar 11, 2021
@jasdel jasdel deleted the fixup/Ec2Model branch May 6, 2021 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/needs-review This PR needs a review from a Member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

service/ec2: ModifyNetworkInterfaceAttribute fails when Value: false but works when Value: true
2 participants