Change timeout settings to merge them when configuration is merged #3405
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. |
aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TimeoutConfigMergingTest.kt
Outdated
Show resolved
Hide resolved
| @@ -51,7 +51,7 @@ fun awsSdkIntegrationTest( | |||
|
|
|||
| fun awsIntegrationTestParams() = | |||
| IntegrationTestParams( | |||
| cargoCommand = "cargo test --features test-util behavior-version-latest", | |||
| cargoCommand = "cargo test --features test-util,behavior-version-latest --tests --lib", | |||
There was a problem hiding this comment.
Are the --tests --lib args actually doing anything?
There was a problem hiding this comment.
yes! they mean we don't run doc tests which changes the test run for 30 seconds to one second
| @@ -60,3 +66,60 @@ async fn timeouts_can_be_set_by_service() { | |||
| // it's shorter than the 5 second timeout if the test is broken | |||
| assert!(start.elapsed() < Duration::from_millis(500)); | |||
| } | |||
|
|
|||
| /// Use a 5 second operation timeout on SdkConfig and a 0ms operation timeout on the service config | |||
There was a problem hiding this comment.
Comment doesn't match reality
aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs
Outdated
Show resolved
Hide resolved
| rustTemplate( | ||
| """ | ||
| pub fn set_timeout_config(&mut self, timeout_config: #{Option}<#{TimeoutConfig}>) -> &mut Self { | ||
| timeout_config.map(|t| self.config.store_put(t)); | ||
| let mut timeout_config = timeout_config.unwrap_or_else(#{TimeoutConfig}::disabled); |
There was a problem hiding this comment.
This seems like unexpected behavior? Setting it to None disables all timeouts?
There was a problem hiding this comment.
oh...that's a good point actually. I thought that was how it used to behave but that's wrong. Will fix. Good catch
| impl Store for MergeTimeoutConfig { | ||
| type ReturnedType<'a> = TimeoutConfig; | ||
| type StoredType = <StoreReplace<TimeoutConfig> as Store>::StoredType; |
There was a problem hiding this comment.
🤯
I had no idea this was possible.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Co-authored-by: John DiSanti <[email protected]>
|
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
For context, see #3408
Description
invoke, load all timeout configs and merge them via a custom loader.T.TimeoutConfig::disabled.Checklist
CHANGELOG.next.tomlif I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.tomlif 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.