-
Notifications
You must be signed in to change notification settings - Fork 79
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
feat(endpoints)!: integrate endpoints 2.0 #607
Conversation
b6f4805
to
dcf193a
Compare
f9aaf38
to
8fea690
Compare
/** | ||
The service name that should be used for signing requests to this endpoint. | ||
This overrides the default signing name used by an SDK client. | ||
*/ | ||
let signingName: String? | ||
public let signingName: String? | ||
/** | ||
The region that should be used for signing requests to this endpoint. | ||
This overrides the default signing region used by an SDK client. | ||
*/ | ||
let signingRegion: String? | ||
public let signingRegion: String? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Aren't these superseded by similar entries in Endpoint.properties
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, they have now become convenience properties. Just removing them will result in a bigger refactor given their references all over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm missing something. Are these pre-existing fields now backed by the properties
bag somehow? How will existing code that utilizes them get valid values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is what you are looking for in a test case https://github.com/awslabs/aws-sdk-swift/blob/jangirg/feat/endpoints-2/codegen/smithy-aws-swift-codegen/src/test/kotlin/software/amazon/smithy/aws/swift/codegen/EndpointResolverMiddlewareTests.kt#L50-L54
I take back the word convenience
rather authScheme
is determined first and then passed to AWSEndpoint
init.
guard let authScheme = endpoint.authScheme(name: "sigv4") else {
throw ClientRuntime.SdkError<OperationStackError>.client(ClientError.unknownError(("Unable to resolve endpoint. Unsupported auth scheme.")))
}
let awsEndpoint = AWSEndpoint(endpoint: endpoint, signingName: authScheme["signingName"] as? String, signingRegion: authScheme["signingRegion"] as? String)
...-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/EndpointParamsGenerator.kt
Outdated
Show resolved
Hide resolved
...-codegen/src/main/kotlin/software/amazon/smithy/aws/swift/codegen/EndpointParamsGenerator.kt
Outdated
Show resolved
Hide resolved
...ain/kotlin/software/amazon/smithy/aws/swift/codegen/middleware/EndpointResolverMiddleware.kt
Outdated
Show resolved
Hide resolved
...n/software/amazon/smithy/aws/swift/codegen/middleware/OperationEndpointResolverMiddleware.kt
Outdated
Show resolved
Hide resolved
...n/software/amazon/smithy/aws/swift/codegen/middleware/OperationEndpointResolverMiddleware.kt
Outdated
Show resolved
Hide resolved
...n/kotlin/software/amazon/smithy/aws/swift/codegen/model/AWSClientContextParamsTransformer.kt
Outdated
Show resolved
Hide resolved
...n/kotlin/software/amazon/smithy/aws/swift/codegen/model/AWSClientContextParamsTransformer.kt
Outdated
Show resolved
Hide resolved
...n/software/amazon/smithy/aws/swift/codegen/middleware/OperationEndpointResolverMiddleware.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed more for training purposes than for correctness or architecture. Interesting to see how types and their tests are generated.
d5c99ed
to
f039f3e
Compare
d801431
to
fa1c4a3
Compare
fed6cc3
to
88d4085
Compare
Issue #
#624
Description of changes
Counterpart: smithy-lang/smithy-swift#433
Endpoint struct
Endpoint struct now supports
attributes
andheaders
properties.EndpointResolver
By default,
EndpointResolver
is a wrapper on top of Common Runtime (CRT) component, but could be overridden by customers using service client configuration that require some variance in how an endpoint is resolved. BothEndpointResolver
andEndpointParameter
are service specific and codegen toAWS<service name>
package.Client & Configuration
Service client initializer accepts service specific configuration
<service name>ClientConfigurationProtocol
instead baseAWSClientRuntime.AWSClientConfiguration
. This is breaking change to existing service clients which take a generic client configuration. This change is required to allow service specific configuration parameters and endpoint resolver.Client configuration initiazlier now updated with alphatic order of the params which is also a breaking change because previously
region
was always the first parameter andruntimeConfig
last. This creates confusing and inconsistent experience comparing with other structs we have in the SDK where params follows alphabatic order make it easier to introduce more params without breaking code.Middleware
Middleware is going to be service specific to handle customization for endpoint parameters creation.
EndpointMiddleware
is codegen in the service package. Endpoint Parameters are service specific which are also codegen.For an hypothetical
PutEvents
operation in an example serviceRequest and response
EndpointResolverMiddleware
EndpointResolverMiddleware
is service specific which takes dependency on service specificEndpointResolver
&EndpointParams
.EndpointMiddleware usage
Similar to other middleware construction,
EndpointResolverMiddlware
is injected before thebuildStep
with extra codegenendpointParams
instance that goes to theinit
.User experience
Case 1: client init with region
This is simplest way to instantiate a service client by just providing the region as a param.
Case 2: client init with config
Customers can instantiate the config with their endpoint specific configurations such as
useDualStack
,useFips
or any client level configuration and pass down to the service client init.Case 3: customer endpoint resolver
This is advanced use case where customers can override the
EndpointResolver
with their own implementation. First thing first,EndpointResolver
protocol must be confirmed by their own implementation.Followed by creating the client config by passing confirming struct/class
New/existing dependencies impact assessment, if applicable
Conventional Commits
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.