Possible improvement on config.LoadDefaultConfig to accept existing config.LoadOptionsFunc type instead of dynamic function signature

[migueleliasweb]

Describe the bug

The function config.LoadDefaultConfig second parameter is typed optFns ...func(*LoadOptions) error instead of using the previouisly defined config.LoadOptionsFunc.

This forces users to redefine the whole type if they want to dinamically use the variadic options.

Expected Behavior

If the function config.LoadDefaultConfig used config.LoadOptionsFunc for its second parameter's type, this would make it a more elegant and concise code.

Current Behavior

If a user tries to use config.LoadOptionsFunc they will get the following message:

cannot use awsConfigOptions (variable of type []"github.com/aws/aws-sdk-go-v2/config".LoadOptionsFunc) as []func(*"github.com/aws/aws-sdk-go-v2/config".LoadOptions) error value in argument to awsConfig.LoadDefaultConfig

Reproduction Steps

// awsConfigOptions := []awsConfig.LoadOptionsFunc{} // this won't work properly as although the types are "the same" golang won't allow a runtime lambda-like function to match agaist a defined type signarture (afaik)
awsConfigOptions := []func(*awsConfig.LoadOptions) error{}

if config.Debug { // let's say config.Debug comes from the user's config
	awsConfigOptions = append(
		awsConfigOptions,
		awsConfig.WithClientLogMode(aws.LogRequestWithBody|aws.LogRequestEventMessage),
	)
}

cfg, err := awsConfig.LoadDefaultConfig(
	context.TODO(),
	awsConfigOptions...,
)

Possible Solution

Make it so the function LoadDefaultConfig accepts a variadic type of []awsConfig.LoadOptionsFunc{}.

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2 v1.18.1

Compiler and Version used

go1.20.1 darwin/arm64

Operating System and version

MacOS

[migueleliasweb]

If that helps, the definition for the function:

func LoadDefaultConfig(ctx context.Context, optFns ...func(*LoadOptions) error) (cfg aws.Config, err error)

seems to be 3 days older (via git blame) than

type LoadOptionsFunc func(*LoadOptions) error.

Maybe this is just a matter of someone missing to come back and reuse the type defined at a later time.

[migueleliasweb]

Also, note that this is not really a bug per se. The code can and will work normally. This is just a possible improvement.

[davidjb]

I've opened a PR to address this at #2199.

[lucix-aws]

Unfortunately the proposed change is an API break since these two types are not interchangeable.

Given the revised function definition:

func LoadDefaultConfig(ctx context.Context, optFns ...func(*LoadOptions) error)

The reverse w.r.t. what is accepted by the call in question becomes true: []config.LoadOptionsFunc{} can be passed to config.LoadDefaultConfig, but []func(*config.LoadOptions) error cannot:

// this now works
awsConfigOptions := []awsConfig.LoadOptionsFunc{}
// this is now broken
// awsConfigOptions := []func(*awsConfig.LoadOptions) error{}

if config.Debug {
	awsConfigOptions = append(
		awsConfigOptions,
		awsConfig.WithClientLogMode(aws.LogRequestWithBody|aws.LogRequestEventMessage),
	)
}

cfg, err := awsConfig.LoadDefaultConfig(
	context.TODO(),
	awsConfigOptions...,
)

Closing since we're unable to make this change.

[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.

[davidjb]

@lucix-aws Is this something that can be addressed in the next major version of the SDK?

[migueleliasweb]

Would making a type alias work, @lucix-aws ?

[lucix-aws]

Is this something that can be addressed in the next major version of the SDK?

@davidjb There won't be a new major semver version of the v2 SDK. The next effective "major version" would be a v3 of the Go SDK, which we currently have no plans to implement.

Would making a type alias work

@migueleliasweb Can you clarify/provide an example of what you're envisioning?

For the record -- I don't really see an issue with the API as it's defined now. Having to re-declare the signature may be slightly cumbersome, but you're still able to use the typedef and the explicit signature interchangeably when working with the actual slice. The example you've provided demonstrates this:

if config.Debug {
	awsConfigOptions = append(
		awsConfigOptions,
		// the return of `WithClientLogMode` is `LoadOptionsFunc`, but you can still pass it to
		// the append call even though the slice is declared as `[]func(*LoadOptions) error`
		awsConfig.WithClientLogMode(aws.LogRequestWithBody|aws.LogRequestEventMessage),
	)
}