-
Notifications
You must be signed in to change notification settings - Fork 193
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
Enable Endpoints 2.0 #2074
Enable Endpoints 2.0 #2074
Conversation
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
@@ -4,6 +4,7 @@ | |||
*/ | |||
|
|||
//! AWS SDK endpoint support. | |||
#![allow(deprecated)] |
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.
We expect to eventually remove the resolver fn right? If yes, we should create an issue and add a TODO here
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.
Actually, now that I think about it, perhaps we just need to create an issue that says "remove all deprecated code" that we pick up before GA
|
||
class AwsEndpointDecorator : RustCodegenDecorator<ClientProtocolGenerator, ClientCodegenContext> { | ||
override val name: String = "AwsEndpoint" | ||
override val order: Byte = 0 | ||
override val order: Byte = -100 |
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.
This is giving me CSS Specificity flashbacks. Perhaps we should introduce a constant that represents "after everything else"?
(I think I got that order correct)
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.
Yeah I think we discussed "first", after, before, last
aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsEndpointDecorator.kt
Outdated
Show resolved
Hide resolved
aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsEndpointDecorator.kt
Show resolved
Hide resolved
aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsEndpointDecorator.kt
Show resolved
Hide resolved
aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs
Show resolved
Hide resolved
visitor.execute() | ||
println("file:///$testDir/src/operation.rs") | ||
return testDir | ||
return clientIntegrationTest(model, additionalDecorators = listOf(codegenDecorator)) { _, _ -> } |
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.
What does the { _, _ -> }
at the end mean? Is that a closure that ignores two args and then does nothing? It looks so weird.
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.
yeah, normally it accepts a lambda. Let me put in a default arg for that, good idea.
A new generated diff is ready to view.
A new doc preview is ready to view. |
e2a4c7f
to
a7b8f52
Compare
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
endpoint_url
setter which is threaded into theSdk::Config
builtIn`endpoint_url
Description
This PR enables Endpoints 2.0 for all services. For Smithy native clients, endpoints 2.0 is enabled but we haven't yet setup a default middleware that would make using that possible.
When endpoint rules are not defined & no resolver is provided, we currently will insert a resolver that always fails. A future PR will change this to panic on config construction but we will delay that until more refactoring and soaking has been done.
Testing
Future PRs will follow up with more integration tests
Checklist
CHANGELOG.next.toml
if I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.toml
if I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.