refactor/regen for upstream endpoint middleware/binding changes #2293
Conversation
service/eventbridge/endpoints.go
Outdated
| return params | ||
| } | ||
|
|
||
| func (in *ActivateEventSourceInput) bindEndpointParams(p *EndpointParameters) { |
There was a problem hiding this comment.
Now that I see all of these empty bindings I'm thinking we probably want to just not generate in that case. The outer bind call could then delegate accordingly.
There was a problem hiding this comment.
Did the above after discussing offline.
lucix-aws
force-pushed
the
feat-sraauth-endpointbindings
branch
from
512ddf0 to
2c3ae91
Compare
service/s3/endpoints.go
Outdated
| } | ||
|
|
||
| type resolveEndpointV2Middleware struct { | ||
| options Options |
There was a problem hiding this comment.
is this the client Options in api_client.go?
reason why im asking is because it seems that there was a previous effort to not pass the client Options around directly in middleware, but rather to create intermediary objects and only copy out what you need from the client Options
https://github.com/aws/aws-sdk-go-v2/blob/main/service/s3/api_client.go#L468
https://github.com/aws/aws-sdk-go-v2/blob/main/service/s3/api_op_GetObject.go#L517
would you be able to comment on whether you think that previous pattern should or should not be followed?
also, why give resolver its own field if it already exists in the same options field
func addResolveEndpointV2Middleware(stack *middleware.Stack, options Options) error {
return stack.Serialize.Insert(&resolveEndpointV2Middleware{
options: options,
resolver: options.EndpointResolverV2,
}, "ResolveEndpoint", middleware.After)
}There was a problem hiding this comment.
Discussed offline - this is more important for exported middlewares or situations where code can't take a dependency on Options directly. For codegenned middleware internal to services, passing options at large is generally fine.
service/s3/endpoints.go
Outdated
| return params | ||
| } | ||
|
|
||
| func (in *AbortMultipartUploadInput) bindEndpointParams(p *EndpointParameters) { |
There was a problem hiding this comment.
just curious, was there a strong reason to put all the operation input receivers for bindEndpointParams here rather than in the operation specfic files where the operation input struct is defined? or was it more of a toss up between "it being endpoints related functionality" and "it being an operation method"?
There was a problem hiding this comment.
Moved after discussing offline.
service/s3/endpoints.go
Outdated
| bindEndpointParams(*EndpointParameters) | ||
| } | ||
|
|
||
| func bindEndpointParams(input endpointParamsBinder, options Options) *EndpointParameters { |
There was a problem hiding this comment.
this does look much simpler. i think when i first implemented it i was trying to follow a pattern common in this SDK of only copying out of Options what you need (see my other comment). but seeing how much simpler this is makes me seem to prefer this more.
Track changes in aws/smithy-go#455 and regenerate s3, s3control, and eventbridge services.
The behavior to disable modeled
AccountIdprefix for v2 endpoint resolution in s3control was ported to a standalone middleware, this was previously a per-op "post-resolution" hook in codegen, which doesn't exist anymore.