Don't use pointers for required properties

[stilvoid]

Is your feature request related to a problem? Please describe.
It's frustrating (and not very Gopher-like) to have to do so much conversion to and from pointer types

Describe the solution you'd like
For required properties of Input and Output structs, do not use pointers.
For example:

// The input for the DescribeChangeSet action.
type DescribeChangeSetInput struct {

    // The name or Amazon Resource Name (ARN) of the change set that you want to
    // describe.
    //
    // This member is required.
    ChangeSetName string

    // A string (provided by the DescribeChangeSet response output) that identifies the
    // next page of information that you want to retrieve.
    NextToken *string

    // If you specified the name of a change set, specify the stack name or ID (ARN) of
    // the change set you want to describe.
    StackName *string
}

This more adequately expresses the optional/required distinction through the API which will mean the compiler will catch mistakes before a developer runs into them.

Describe alternatives you've considered
None. Any other solution would be more complex and this change would add a significant simplification imo

Additional context
Add any other context or screenshots about the feature request here.

[stilvoid]

I am aware that this topic has been discussed before and the result was that the change would not be made because it would be a breaking change.

That conversation was a year ago and this SDK has had a major breaking changes since that time so I think it's worth opening the conversation again. Two other facts also make this less of a problem: 1) This repository carries a warning that it's in beta and changes are likely, 2) go modules mean that customer code is pinned to a version of this SDK and only a developer manually updating their dependency will expose them to the change.

This would be a relatively simple, one time breaking change to fix a consistent customer complaint I've seen repeated in a number of media :)

[jasdel]

Thanks for reaching out @stilvoid with this feature request. One of the biggest reasons the v1 and v2 SDK's have not been able to adopted using value types for required members, is not so much that it would create breaking change now in the v2 SDK, but the likely breaking changes later after the SDK has released.

One of the aspects governing AWS API models is that while required member cannot be added to a model within a major version, required can be removed from a member within a major version. This means that while today an API has a set of required members, that API could later be updated making those members no longer required. Making them optional. This change would force a type change in the SDK (e.g. string -> *string) if the SDK were to render member types based on if the parameter is required or not.

With that said the v2 SDK does have new modeling metadata allowing it to render some members a value types instead of pointers types for everything. In the API model this is represented by boxed(pointer type) vs unboxed (value type). This was recently added to the SDK via #887. This allows the SDK to generate list and map members, booleans, and numbers as value types instead of pointers by default. The API model can specify if these members should be treated as boxed (pointer type) parameters.

For example the Amazon EC2 API ModifyTrafficMirrorSession's input struct updated to use values for bool, numbers, and lists.

aws-sdk-go-v2/service/ec2/api_op_ModifyTrafficMirrorSession.go

Lines 30 to 72 in 979cf22

	type ModifyTrafficMirrorSessionInput struct {
	// The ID of the Traffic Mirror session.
	//
	// This member is required.
	TrafficMirrorSessionId *string
	// The description to assign to the Traffic Mirror session.
	Description *string
	// Checks whether you have the required permissions for the action, without
	// actually making the request, and provides an error response. If you have the
	// required permissions, the error response is DryRunOperation. Otherwise, it is
	// UnauthorizedOperation.
	DryRun bool
	// The number of bytes in each packet to mirror. These are bytes after the VXLAN
	// header. To mirror a subset, set this to the length (in bytes) to mirror. For
	// example, if you set this value to 100, then the first 100 bytes that meet the
	// filter criteria are copied to the target. Do not specify this parameter when you
	// want to mirror the entire packet.
	PacketLength int32
	// The properties that you want to remove from the Traffic Mirror session. When you
	// remove a property from a Traffic Mirror session, the property is set to the
	// default.
	RemoveFields []types.TrafficMirrorSessionField
	// The session number determines the order in which sessions are evaluated when an
	// interface is used by multiple sessions. The first session with a matching filter
	// is the one that mirrors the packets. Valid values are 1-32766.
	SessionNumber int32
	// The ID of the Traffic Mirror filter.
	TrafficMirrorFilterId *string
	// The Traffic Mirror target. The target must be in the same VPC as the source, or
	// have a VPC peering connection with the source.
	TrafficMirrorTargetId *string
	// The virtual network ID of the Traffic Mirror session.
	VirtualNetworkId int32
	}

In addition to list, maps, booleans, and number types, the SDK will also generate all enum parameters as value types instead of strings.

aws-sdk-go-v2/service/ec2/types/types.go

Lines 9261 to 9262 in 979cf22

	// The type of Traffic Mirror target.
	Type TrafficMirrorTargetType

The list, map, bool, and number change hasn't been included in a tagged release yet, but will be when the the next release is published.

[stilvoid]

Thanks for the response @jasdel :) The recent change certainly looks like it goes some way towards improving things 👍 I still think we have an opportunity to improve things even further though.

Given that each service is now an independently-versioned go module, we have a mechanism to flag and protect users from breaking changes such as an API dropping the "required" flag for a parameter. The Go SDK doesn't have to follow the same versioning scheme as the API.

As a developer of some application using AWS APIs, I think I'd prefer to be notified (by way of my code breaking when I update my dependencies) that a parameter is no longer required rather than the value simply disappearing into a black hole.

Appreciate you will disagree, so just saying thanks for the considered response :)

[github-actions]

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