-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Some Possible Options for fixing aws.Config.Merge #276
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
Comments
Closed
Closed
jasdel
added a commit
that referenced
this issue
Jul 14, 2015
This change refactors the SDK so that Config fields are pointers instead of values. Primiarly this change fixes the bug preventing setting Config fields to the field's zero value when creating new service client instances. ```go svc := s3.New(&aws.Config{ Region: aws.String("us-west-2"), MaxRetries: aws.Int(10), }) ``` Additionally builder pattern `WithX` methods were added to Config. These methods can be chained so inline building of Config objects can be done without using pointers for the fields. ```go svc := s3.New(aws.NewConfig().WithRegion("us-west-2").WithMaxRetries(10)) ``` Fixes #157 Fixes #276 Fixes #124
jasdel
added a commit
that referenced
this issue
Jul 14, 2015
This change refactors the SDK so that Config fields are pointers instead of values. Primarily this change fixes the bug preventing setting Config fields to the field's zero value when creating new service client instances. ```go svc := s3.New(&aws.Config{ Region: aws.String("us-west-2"), MaxRetries: aws.Int(10), }) ``` Additionally builder pattern `WithX` methods were added to Config. These methods can be chained so inline building of Config objects can be done without using pointers for the fields. ```go svc := s3.New(aws.NewConfig().WithRegion("us-west-2").WithMaxRetries(10)) ``` Fixes #157 Fixes #276 Fixes #124
jasdel
added a commit
that referenced
this issue
Jul 14, 2015
This change refactors the SDK so that Config fields are pointers instead of values. Primarily this change fixes the bug preventing setting Config fields to the field's zero value when creating new service client instances. ```go svc := s3.New(&aws.Config{ Region: aws.String("us-west-2"), MaxRetries: aws.Int(10), }) ``` Additionally builder pattern `WithX` methods were added to Config. These methods can be chained so inline building of Config objects can be done without using pointers for the fields. ```go svc := s3.New(aws.NewConfig().WithRegion("us-west-2").WithMaxRetries(10)) ``` Fixes #157 Fixes #276 Fixes #124
jasdel
added a commit
that referenced
this issue
Jul 20, 2015
This change refactors the SDK so that Config fields are pointers instead of values. Primarily this change fixes the bug preventing setting Config fields to the field's zero value when creating new service client instances. ```go svc := s3.New(&aws.Config{ Region: aws.String("us-west-2"), MaxRetries: aws.Int(10), }) ``` Additionally builder pattern `WithX` methods were added to Config. These methods can be chained so inline building of Config objects can be done without using pointers for the fields. ```go svc := s3.New(aws.NewConfig().WithRegion("us-west-2").WithMaxRetries(10)) ``` Fixes #157 Fixes #276 Fixes #124
jasdel
added a commit
that referenced
this issue
Jul 24, 2015
This change refactors the SDK so that Config fields are pointers instead of values. Primarily this change fixes the bug preventing setting Config fields to the field's zero value when creating new service client instances. ```go svc := s3.New(&aws.Config{ Region: aws.String("us-west-2"), MaxRetries: aws.Int(10), }) ``` Additionally builder pattern `WithX` methods were added to Config. These methods can be chained so inline building of Config objects can be done without using pointers for the fields. ```go svc := s3.New(aws.NewConfig().WithRegion("us-west-2").WithMaxRetries(10)) ``` Fixes #157 Fixes #276 Fixes #124
jasdel
added a commit
that referenced
this issue
Jul 28, 2015
Closed
skotambkar
pushed a commit
to skotambkar/aws-sdk-go
that referenced
this issue
May 20, 2021
Fixes the SDK's UserAgent key for the execution environment. Fix aws#276
skotambkar
pushed a commit
to skotambkar/aws-sdk-go
that referenced
this issue
May 20, 2021
Services --- * Synced the V2 SDK with latest AWS service API definitions. * Fixes aws#304 * Fixes aws#295 SDK Breaking changes --- This update includes multiple breaking changes to the SDK. These updates improve the SDK's usability, consistency. Client type name --- The API client type is renamed to `Client` for consistency, and remove stutter between package and client type name. Using Amazon S3 API client as an example, the `s3.S3` type is renamed to `s3.Client`. New API operation response type --- API operations' `Request.Send` method now returns a Response type for the specific operation. The Response type wraps the operation's Output parameter, and includes a method for the response's metadata such as RequestID. The Output type is an anonymous embedded field within the Output type. If your application was passing the Output value around you'll need to extract it directly, or pass the Response type instead. New API operation paginator utility --- This change removes the `Paginate` method from API operation Request types, (e.g. ListObjectsRequest). A new Paginator constructor is added that can be used to page these operations. To update your application to use the new pattern, where `Paginate` was being called, replace this with the Paginator type's constructor. The usage of the returned Paginator type is unchanged. ```go req := svc.ListObjectsRequest(params) p := req.Paginate() ``` Is updated to to use the Paginator constructor instead of Paginate method. ```go req := svc.ListObjectsRequest(params) p := s3.NewListObjectsPaginator(req) ``` Other changes --- * Standardizes API client package name to be based on the API model's `ServiceID`. * Standardizes API client operation input and output type names. * Removes `endpoints` package's service identifier constants. These values were unstable. Each API client package contains an `EndpointsID` constant that can be used for service specific endpoint lookup. * Fix API endpoint lookups to use the API's modeled `EndpointsID` (aka `enpdointPrefix`). Searching for API endpoints in the `endpoints` package should use the API client package's, `EndpointsID`. SDK Enhancements --- * Update CI tests to ensure all codegen changes are accounted for in PR (aws#183) * Updates the CI tests to ensure that any code generation changes are accounted for in the PR, and that there were no mistaken changes made without also running code generation. This change should also help ensure that code generation order is stable, and there are no ordering issues with the SDK's codegen. * Related aws#1966 * `service/dynamodb/expression`: Fix Builder with KeyCondition example (aws#306) * Fixes the ExampleBuilder_WithKeyCondition example to include the ExpressionAttributeNames member being set. * Fixes aws#285 * `aws/defaults`: Fix UserAgent execution environment key (aws#307) * Fixes the SDK's UserAgent key for the execution environment. * Fixes aws#276 * `private/model/api`: Improve SDK API reference doc generation (aws#309) * Improves the SDK's generated documentation for API client, operation, and types. This fixes several bugs in the doc generation causing poor formatting, an difficult to read reference documentation. * Fix aws#308 * Related aws#2617
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I just pushed two branches for some possible ways we could resolve the aws.Config.Merge problem of not being able to clear fields which were set. For example DisableSSL can never be set to false with Merge() e.g.
<service>.New(&aws.Config{...})
. This issue applies to all non pointer types in aws.Config. Issue #157Both of the two solutions I am proposing are breaking changes, because they make modifications to the Config struct, and the aws.String() helper functions. The goal of these changes are to ensure configurations can be merged correctly without oddities such as not being able to set a field to its zero value because the zero value is used to determine if the incoming configuration parameter is set.
Both proposed solutions renamed aws.String to aws.StringPtr to clarify what aws.String function does. This applied to the other pointer helper functions as well. An IntPtr was added for *int. In addition helper functions were added for each type to return the value that a pointer points to or the type's zero value if the pointer is nil. This saves some of the hassle of checking if a field is nil before dereferencing it in circumstances where nil should be treated as zero. (addresses #124)
We are looking for feedback from the community on what you think of these two methods. Both methods have their strengths and weaknesses, but we would like to fix this pain point.
The best method to view the code of these changes is via the branch. Viewing the commit on Github probably will not be a pleasant experience due to the number of changes.
ConfigMergeViaPointer: Config use pointers instead of values
Switches all Config field primitive types to be pointers. This change is the simplest idea to implement and understand, but will create pain points when using the fields because of the need to nil check before each use. Some of the pain points can be mitigated by using the PtrValue helper method to access the pointer's value or zero value if nil.
Pros:
Cons:
Example:
ConfigMergeViaStruct: Config use custom types instead of primitives
Switches all Config field primitive types to custom wrapper types. The custom types wrap the value and the state if the field was set or not. Constructor helper methods were created for each type wrapper to make it simpler to set field values without multiple lines of code.
Pros:
Cons:
.Get()
Example:
The text was updated successfully, but these errors were encountered: