diff --git a/CHANGELOG.md b/CHANGELOG.md index adc6e18456a61a53884f4fc7de064846babc18ad..73cd86114e7fe82cdb6674397c63a3b10c8ebea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ v0.25 (October 7th, 2021) ========================= **Breaking changes** - :warning: MSRV increased from 1.52.1 to 1.53.0 per our 3-behind MSRV policy. +- :warning: `smithy_client::retry::Config` field `max_retries` is renamed to `max_attempts` + - This also brings a change to the semantics of the field. In the old version, setting `max_retries` to 3 would mean + that up to 4 requests could occur (1 initial request and 3 retries). In the new version, setting `max_attempts` to 3 + would mean that up to 3 requests could occur (1 initial request and 2 retries). +- :warning: `smithy_client::retry::Config::with_max_retries` method is renamed to `with_max_attempts` - :warning: Several classes in the codegen module were renamed and/or refactored (smithy-rs#735): - `ProtocolConfig` became `CodegenContext` and moved to `software.amazon.smithy.rust.codegen.smithy` - `HttpProtocolGenerator` became `ProtocolGenerator` and was refactored @@ -22,7 +27,9 @@ v0.25 (October 7th, 2021) - The `DispatchError` variant of `SdkError` now contains `ConnectorError` instead of `Box` (#744). **New this week** + - :bug: Fix an issue where `smithy-xml` may have generated invalid XML (smithy-rs#719) +- Add `RetryConfig` struct for configuring retry behavior (smithy-rs#725) - :bug: Fix error when receiving empty event stream messages (smithy-rs#736) - :bug: Fix bug in event stream receiver that could cause the last events in the response stream to be lost (smithy-rs#736) - Add connect & HTTP read timeouts to IMDS, defaulting to 1 second diff --git a/aws/SDK_CHANGELOG.md b/aws/SDK_CHANGELOG.md index afefe340d16503f519123986965cd0a9d332b2d7..ab164304220c2b72e1a16d0dedfe23ede761fb73 100644 --- a/aws/SDK_CHANGELOG.md +++ b/aws/SDK_CHANGELOG.md @@ -2,15 +2,26 @@ vNext (Month Day, Year) ======================= **Breaking changes** + - :warning: MSRV increased from 1.52.1 to 1.53.0 per our 3-behind MSRV policy. - `SmithyConnector` and `DynConnector` now return `ConnectorError` instead of `Box`. If you have written a custom connector, it will need to be updated to return the new error type. (#744) - The `DispatchError` variant of `SdkError` now contains `ConnectorError` instead of `Box` (#744). **Tasks to cut release** + - [ ] Bump MSRV on aws-sdk-rust, then delete this line. **New This Week** +- :tada: Make retry behavior configurable + - With env vars `AWS_MAX_ATTEMPTS` and `AWS_RETRY_MODE` + - With `~/.aws/config` settings `max_attempts` and `retry_mode` + - By calling the `with_retry_config` method on a `Config` and passing in a `RetryConfig` + - Only the `Standard` retry mode is currently implemented. `Adaptive` retry mode will be implemented at a later + date. + - For more info, see the AWS Reference pages on configuring these settings: + - [Setting global max attempts](https://docs.aws.amazon.com/sdkref/latest/guide/setting-global-max_attempts.html) + - [Setting global retry mode](https://docs.aws.amazon.com/sdkref/latest/guide/setting-global-retry_mode.html) - :tada: Add presigned request support and examples for S3 GetObject and PutObject (smithy-rs#731, aws-sdk-rust#139) - :tada: Add presigned request support and example for Polly SynthesizeSpeech (smithy-rs#735, aws-sdk-rust#139) - Add connect & HTTP read timeouts to IMDS, defaulting to 1 second diff --git a/aws/rust-runtime/aws-config/Cargo.toml b/aws/rust-runtime/aws-config/Cargo.toml index d35f6032db35728d0ef9cb16151b4a0827c7e50d..a8dde5aeab73abf4b178100a487c0e75e4c26ab9 100644 --- a/aws/rust-runtime/aws-config/Cargo.toml +++ b/aws/rust-runtime/aws-config/Cargo.toml @@ -10,7 +10,7 @@ exclude = ["test-data/*", "integration-tests/*"] default-provider = ["profile", "imds", "meta", "sts", "environment"] profile = ["sts", "web-identity-token", "meta", "environment", "imds"] meta = ["tokio/sync"] -imds = ["profile", "smithy-http", "smithy-types", "smithy-http-tower", "smithy-json", "tower", "aws-http", "meta"] +imds = ["profile", "smithy-http", "smithy-http-tower", "smithy-json", "tower", "aws-http", "meta"] environment = ["meta"] sts = ["aws-sdk-sts", "aws-hyper"] web-identity-token = ["sts", "profile"] @@ -28,6 +28,7 @@ default = ["default-provider", "rustls", "rt-tokio"] aws-types = { path = "../../sdk/build/aws-sdk/aws-types" } smithy-async = { path = "../../sdk/build/aws-sdk/smithy-async" } smithy-client = { path = "../../sdk/build/aws-sdk/smithy-client" } +smithy-types = { path = "../../sdk/build/aws-sdk/smithy-types" } tracing = { version = "0.1" } tokio = { version = "1", features = ["sync"], optional = true } aws-sdk-sts = { path = "../../sdk/build/aws-sdk/sts", optional = true } @@ -37,7 +38,6 @@ aws-hyper = { path = "../../sdk/build/aws-sdk/aws-hyper", optional = true } # imds smithy-http = { path = "../../sdk/build/aws-sdk/smithy-http", optional = true } -smithy-types = { path = "../../sdk/build/aws-sdk/smithy-types", optional = true } smithy-http-tower = { path = "../../sdk/build/aws-sdk/smithy-http-tower", optional = true } tower = { version = "0.4.8", optional = true } aws-http = { path = "../../sdk/build/aws-sdk/aws-http", optional = true } diff --git a/aws/rust-runtime/aws-config/src/default_provider.rs b/aws/rust-runtime/aws-config/src/default_provider.rs index dd9e4c7afe7dd5804c29a77bb3beac71314ae910..4c2fbb54eeb2af65be42ae48eacdddb4dfbf4a05 100644 --- a/aws/rust-runtime/aws-config/src/default_provider.rs +++ b/aws/rust-runtime/aws-config/src/default_provider.rs @@ -7,18 +7,15 @@ //! //! Unless specific configuration is required, these should be constructed via [`ConfigLoader`](crate::ConfigLoader). //! -//! /// Default region provider chain pub mod region { + use aws_types::region::Region; use crate::environment::region::EnvironmentVariableRegionProvider; use crate::meta::region::{ProvideRegion, RegionProviderChain}; - use crate::{imds, profile}; - use crate::provider_config::ProviderConfig; - - use aws_types::region::Region; + use crate::{imds, profile}; /// Default Region Provider chain /// @@ -89,17 +86,109 @@ pub mod region { } } +/// Default retry behavior configuration provider chain +pub mod retry_config { + use smithy_types::retry::RetryConfig; + + use crate::environment::retry_config::EnvironmentVariableRetryConfigProvider; + use crate::profile; + use crate::provider_config::ProviderConfig; + + /// Default RetryConfig Provider chain + /// + /// Unlike other "providers" `RetryConfig` has no related `RetryConfigProvider` trait. Instead, + /// a builder struct is returned which has a similar API. + /// + /// This provider will check the following sources in order: + /// 1. [Environment variables](EnvironmentVariableRetryConfigProvider) + /// 2. [Profile file](crate::profile::retry_config::ProfileFileRetryConfigProvider) + /// + /// # Example + /// + /// ```rust + /// # use std::error::Error; + /// # #[tokio::main] + /// # async fn main() -> Result<(), Box> { + /// use aws_config::default_provider::retry_config; + /// // Creating a RetryConfig from the default_provider already happens when loading a config from_env + /// // This is only for illustration purposes + /// let retry_config = retry_config::default_provider().retry_config().await; + /// let config = aws_config::from_env().retry_config(retry_config).load().await; + /// // instantiate a service client: + /// // ::Client::new(&config); + /// # Ok(()) + /// # } + /// ``` + pub fn default_provider() -> Builder { + Builder::default() + } + + /// Builder for RetryConfig that checks the environment and aws profile for configuration + #[derive(Default)] + pub struct Builder { + env_provider: EnvironmentVariableRetryConfigProvider, + profile_file: profile::retry_config::Builder, + } + + impl Builder { + #[doc(hidden)] + /// Configure the default chain + /// + /// Exposed for overriding the environment when unit-testing providers + pub fn configure(mut self, configuration: &ProviderConfig) -> Self { + self.env_provider = + EnvironmentVariableRetryConfigProvider::new_with_env(configuration.env()); + self.profile_file = self.profile_file.configure(configuration); + self + } + + /// Override the profile name used by this provider + pub fn profile_name(mut self, name: &str) -> Self { + self.profile_file = self.profile_file.profile_name(name); + self + } + + /// Attempt to create a [RetryConfig](smithy_types::retry::RetryConfig) from following sources in order: + /// 1. [Environment variables](crate::environment::retry_config::EnvironmentVariableRetryConfigProvider) + /// 2. [Profile file](crate::profile::retry_config::ProfileFileRetryConfigProvider) + /// 3. [RetryConfig::default()](smithy_types::retry::RetryConfig::default) + /// + /// Precedence is considered on a per-field basis + /// + /// # Panics + /// + /// - Panics if the `AWS_MAX_ATTEMPTS` env var or `max_attempts` profile var is set to 0 + /// - Panics if the `AWS_RETRY_MODE` env var or `retry_mode` profile var is set to "adaptive" (it's not yet supported) + pub async fn retry_config(self) -> RetryConfig { + // Both of these can return errors due to invalid config settings and we want to surface those as early as possible + // hence, we'll panic if any config values are invalid (missing values are OK though) + // We match this instead of unwrapping so we can print the error with the `Display` impl instead of the `Debug` impl that unwrap uses + let builder_from_env = match self.env_provider.retry_config_builder() { + Ok(retry_config_builder) => retry_config_builder, + Err(err) => panic!("{}", err), + }; + let builder_from_profile = match self.profile_file.build().retry_config_builder().await + { + Ok(retry_config_builder) => retry_config_builder, + Err(err) => panic!("{}", err), + }; + + builder_from_env.merge_with(builder_from_profile).build() + } + } +} + /// Default credentials provider chain pub mod credentials { + use std::borrow::Cow; + + use aws_types::credentials::{future, ProvideCredentials}; + use crate::environment::credentials::EnvironmentVariableCredentialsProvider; use crate::meta::credentials::{CredentialsProviderChain, LazyCachingCredentialsProvider}; use crate::meta::region::ProvideRegion; - use aws_types::credentials::{future, ProvideCredentials}; - use crate::provider_config::ProviderConfig; - use std::borrow::Cow; - #[cfg(any(feature = "rustls", feature = "native-tls"))] /// Default Credentials Provider chain /// @@ -239,10 +328,11 @@ pub mod credentials { Some(provider) => provider.region().await, None => self.region_chain.build().region().await, }; + let conf = self.conf.unwrap_or_default().with_region(region); - let profile_provider = self.profile_file_builder.configure(&conf).build(); let env_provider = EnvironmentVariableCredentialsProvider::new_with_env(conf.env()); + let profile_provider = self.profile_file_builder.configure(&conf).build(); let web_identity_token_provider = self.web_identity_builder.configure(&conf).build(); let imds_provider = self.imds_builder.configure(&conf).build(); @@ -251,12 +341,23 @@ pub mod credentials { .or_else("WebIdentityToken", web_identity_token_provider) .or_else("Ec2InstanceMetadata", imds_provider); let cached_provider = self.credential_cache.configure(&conf).load(provider_chain); + DefaultCredentialsChain(cached_provider.build()) } } #[cfg(test)] mod test { + use tracing_test::traced_test; + + use aws_types::credentials::ProvideCredentials; + use aws_types::os_shim_internal::{Env, Fs}; + use smithy_types::retry::{RetryConfig, RetryMode}; + + use crate::default_provider::credentials::DefaultCredentialsChain; + use crate::default_provider::retry_config; + use crate::provider_config::ProviderConfig; + use crate::test_case::TestEnvironment; /// Test generation macro /// @@ -307,11 +408,6 @@ pub mod credentials { }; } - use crate::default_provider::credentials::DefaultCredentialsChain; - use crate::test_case::TestEnvironment; - use aws_types::credentials::ProvideCredentials; - use tracing_test::traced_test; - make_test!(prefer_environment); make_test!(profile_static_keys); make_test!(web_identity_token_env); @@ -347,5 +443,123 @@ pub mod credentials { .expect("creds should load"); assert_eq!(creds.access_key_id(), "correct_key_secondary"); } + + #[tokio::test] + async fn test_returns_default_retry_config_from_empty_profile() { + let env = Env::from_slice(&[("AWS_CONFIG_FILE", "config")]); + let fs = Fs::from_slice(&[("config", "[default]\n")]); + + let provider_config = ProviderConfig::no_configuration().with_env(env).with_fs(fs); + + let actual_retry_config = retry_config::default_provider() + .configure(&provider_config) + .retry_config() + .await; + + let expected_retry_config = RetryConfig::new(); + + assert_eq!(actual_retry_config, expected_retry_config); + // This is redundant but it's really important to make sure that + // we're setting these exact values by default so we check twice + assert_eq!(actual_retry_config.max_attempts(), 3); + assert_eq!(actual_retry_config.mode(), RetryMode::Standard); + } + + #[tokio::test] + async fn test_no_retry_config_in_empty_profile() { + let env = Env::from_slice(&[("AWS_CONFIG_FILE", "config")]); + let fs = Fs::from_slice(&[("config", "[default]\n")]); + + let provider_config = ProviderConfig::no_configuration().with_env(env).with_fs(fs); + + let actual_retry_config = retry_config::default_provider() + .configure(&provider_config) + .retry_config() + .await; + + let expected_retry_config = RetryConfig::new(); + + assert_eq!(actual_retry_config, expected_retry_config) + } + + #[tokio::test] + async fn test_creation_of_retry_config_from_profile() { + let env = Env::from_slice(&[("AWS_CONFIG_FILE", "config")]); + // TODO standard is the default mode; this test would be better if it was setting it to adaptive mode + // adaptive mode is currently unsupported so that would panic + let fs = Fs::from_slice(&[( + "config", + // If the lines with the vars have preceding spaces, they don't get read + r#"[default] +max_attempts = 1 +retry_mode = standard + "#, + )]); + + let provider_config = ProviderConfig::no_configuration().with_env(env).with_fs(fs); + + let actual_retry_config = retry_config::default_provider() + .configure(&provider_config) + .retry_config() + .await; + + let expected_retry_config = RetryConfig::new() + .with_max_attempts(1) + .with_retry_mode(RetryMode::Standard); + + assert_eq!(actual_retry_config, expected_retry_config) + } + + #[tokio::test] + async fn test_env_retry_config_takes_precedence_over_profile_retry_config() { + let env = Env::from_slice(&[ + ("AWS_CONFIG_FILE", "config"), + ("AWS_MAX_ATTEMPTS", "42"), + ("AWS_RETRY_MODE", "standard"), + ]); + // TODO standard is the default mode; this test would be better if it was setting it to adaptive mode + // adaptive mode is currently unsupported so that would panic + let fs = Fs::from_slice(&[( + "config", + // If the lines with the vars have preceding spaces, they don't get read + r#"[default] +max_attempts = 88 +retry_mode = standard + "#, + )]); + + let provider_config = ProviderConfig::no_configuration().with_env(env).with_fs(fs); + + let actual_retry_config = retry_config::default_provider() + .configure(&provider_config) + .retry_config() + .await; + + let expected_retry_config = RetryConfig::new() + .with_max_attempts(42) + .with_retry_mode(RetryMode::Standard); + + assert_eq!(actual_retry_config, expected_retry_config) + } + + #[tokio::test] + #[should_panic = "failed to parse max attempts set by aws profile: invalid digit found in string"] + async fn test_invalid_profile_retry_config_panics() { + let env = Env::from_slice(&[("AWS_CONFIG_FILE", "config")]); + let fs = Fs::from_slice(&[( + "config", + // If the lines with the vars have preceding spaces, they don't get read + r#"[default] +max_attempts = potato + "#, + )]); + + let provider_config = ProviderConfig::no_configuration().with_env(env).with_fs(fs); + + let _ = retry_config::default_provider() + .configure(&provider_config) + .retry_config() + .await; + } } } diff --git a/aws/rust-runtime/aws-config/src/environment/mod.rs b/aws/rust-runtime/aws-config/src/environment/mod.rs index 78c59447d6eb2bc12727cac1d1338890f5904456..525765de65b849c4b071c3a61cb187d6a956f666 100644 --- a/aws/rust-runtime/aws-config/src/environment/mod.rs +++ b/aws/rust-runtime/aws-config/src/environment/mod.rs @@ -9,3 +9,7 @@ pub use credentials::EnvironmentVariableCredentialsProvider; /// Load regions from the environment pub mod region; pub use region::EnvironmentVariableRegionProvider; + +/// Load retry behavior configuration from the environment +pub mod retry_config; +pub use retry_config::EnvironmentVariableRetryConfigProvider; diff --git a/aws/rust-runtime/aws-config/src/environment/retry_config.rs b/aws/rust-runtime/aws-config/src/environment/retry_config.rs new file mode 100644 index 0000000000000000000000000000000000000000..b8ac84fdba53c2188fe3fe1e43251c25296e6c1d --- /dev/null +++ b/aws/rust-runtime/aws-config/src/environment/retry_config.rs @@ -0,0 +1,157 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +use std::str::FromStr; + +use aws_types::os_shim_internal::Env; +use smithy_types::retry::{RetryConfigBuilder, RetryConfigErr, RetryMode}; + +const ENV_VAR_MAX_ATTEMPTS: &str = "AWS_MAX_ATTEMPTS"; +const ENV_VAR_RETRY_MODE: &str = "AWS_RETRY_MODE"; + +/// Load a retry_config from environment variables +/// +/// This provider will check the values of `AWS_RETRY_MODE` and `AWS_MAX_ATTEMPTS` +/// in order to build a retry config. If at least one is set to a valid value, +/// construction will succeed +#[derive(Debug, Default)] +pub struct EnvironmentVariableRetryConfigProvider { + env: Env, +} + +impl EnvironmentVariableRetryConfigProvider { + /// Create a new `EnvironmentVariableRetryConfigProvider` + pub fn new() -> Self { + EnvironmentVariableRetryConfigProvider { env: Env::real() } + } + + #[doc(hidden)] + /// Create an retry_config provider from a given `Env` + /// + /// This method is used for tests that need to override environment variables. + pub fn new_with_env(env: Env) -> Self { + EnvironmentVariableRetryConfigProvider { env } + } + + /// Attempt to create a new `RetryConfig` from environment variables + pub fn retry_config_builder(&self) -> Result { + let max_attempts = match self.env.get(ENV_VAR_MAX_ATTEMPTS).ok() { + Some(max_attempts) => match max_attempts.parse::() { + Ok(max_attempts) if max_attempts == 0 => { + return Err(RetryConfigErr::MaxAttemptsMustNotBeZero { + set_by: "environment variable".into(), + }); + } + Ok(max_attempts) => Some(max_attempts), + Err(source) => { + return Err(RetryConfigErr::FailedToParseMaxAttempts { + set_by: "environment variable".into(), + source, + }); + } + }, + None => None, + }; + + let retry_mode = match self.env.get(ENV_VAR_RETRY_MODE) { + Ok(retry_mode) => match RetryMode::from_str(&retry_mode) { + Ok(retry_mode) => Some(retry_mode), + Err(retry_mode_err) => { + return Err(RetryConfigErr::InvalidRetryMode { + set_by: "environment variable".into(), + source: retry_mode_err, + }); + } + }, + Err(_) => None, + }; + + let mut retry_config_builder = RetryConfigBuilder::new(); + retry_config_builder + .set_max_attempts(max_attempts) + .set_mode(retry_mode); + + Ok(retry_config_builder) + } +} + +#[cfg(test)] +mod test { + use aws_types::os_shim_internal::Env; + use smithy_types::retry::{RetryConfig, RetryConfigErr, RetryMode}; + + use super::{EnvironmentVariableRetryConfigProvider, ENV_VAR_MAX_ATTEMPTS, ENV_VAR_RETRY_MODE}; + + fn test_provider(vars: &[(&str, &str)]) -> EnvironmentVariableRetryConfigProvider { + EnvironmentVariableRetryConfigProvider::new_with_env(Env::from_slice(vars)) + } + + #[test] + fn no_retry_config() { + let builder = test_provider(&[]).retry_config_builder().unwrap(); + + assert_eq!(builder.mode, None); + assert_eq!(builder.max_attempts, None); + } + + #[test] + fn max_attempts_is_read_correctly() { + assert_eq!( + test_provider(&[(ENV_VAR_MAX_ATTEMPTS, "88")]) + .retry_config_builder() + .unwrap() + .build(), + RetryConfig::new().with_max_attempts(88) + ); + } + + #[test] + fn max_attempts_errors_when_it_cant_be_parsed_as_an_integer() { + assert!(matches!( + test_provider(&[(ENV_VAR_MAX_ATTEMPTS, "not an integer")]) + .retry_config_builder() + .unwrap_err(), + RetryConfigErr::FailedToParseMaxAttempts { .. } + )); + } + + #[test] + fn retry_mode_is_read_correctly() { + assert_eq!( + test_provider(&[(ENV_VAR_RETRY_MODE, "standard")]) + .retry_config_builder() + .unwrap() + .build(), + RetryConfig::new().with_retry_mode(RetryMode::Standard) + ); + } + + #[test] + fn both_fields_can_be_set_at_once() { + assert_eq!( + test_provider(&[ + (ENV_VAR_RETRY_MODE, "standard"), + (ENV_VAR_MAX_ATTEMPTS, "13") + ]) + .retry_config_builder() + .unwrap() + .build(), + RetryConfig::new() + .with_max_attempts(13) + .with_retry_mode(RetryMode::Standard) + ); + } + + #[test] + fn disallow_zero_max_attempts() { + let err = test_provider(&[(ENV_VAR_MAX_ATTEMPTS, "0")]) + .retry_config_builder() + .unwrap_err(); + assert!(matches!( + err, + RetryConfigErr::MaxAttemptsMustNotBeZero { .. } + )); + } +} diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 41b6063d40551ef12912e063ccb267ac5870bc3b..48bece80b1e99d933e2850d0adaba74712575267 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -46,7 +46,7 @@ mod token; // 6 hours const DEFAULT_TOKEN_TTL: Duration = Duration::from_secs(21_600); -const DEFAULT_RETRIES: u32 = 3; +const DEFAULT_ATTEMPTS: u32 = 4; const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_secs(1); const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(1); @@ -353,7 +353,7 @@ impl EndpointMode { /// IMDSv2 Client Builder #[derive(Default, Debug, Clone)] pub struct Builder { - num_retries: Option, + max_attempts: Option, endpoint: Option, mode_override: Option, token_ttl: Option, @@ -399,9 +399,9 @@ impl Error for BuildError { impl Builder { /// Override the number of retries for fetching tokens & metadata /// - /// By default, 3 retries will be made. - pub fn retries(mut self, retries: u32) -> Self { - self.num_retries = Some(retries); + /// By default, 4 attempts will be made. + pub fn max_attempts(mut self, max_attempts: u32) -> Self { + self.max_attempts = Some(max_attempts); self } @@ -493,8 +493,8 @@ impl Builder { .unwrap_or_else(|| EndpointSource::Env(config.env(), config.fs())); let endpoint = endpoint_source.endpoint(self.mode_override).await?; let endpoint = Endpoint::immutable(endpoint); - let retry_config = - retry::Config::default().with_max_retries(self.num_retries.unwrap_or(DEFAULT_RETRIES)); + let retry_config = retry::Config::default() + .with_max_attempts(self.max_attempts.unwrap_or(DEFAULT_ATTEMPTS)); let token_loader = token::TokenMiddleware::new( connector.clone(), config.time_source(), diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index 68245e265e65a80ad1eecda8b85c52217f72fb5a..903c2ec2538690557f599bcf2027b5b308999b27 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -107,10 +107,11 @@ pub use loader::ConfigLoader; #[cfg(feature = "default-provider")] mod loader { - use crate::default_provider::{credentials, region}; + use crate::default_provider::{credentials, region, retry_config}; use crate::meta::region::ProvideRegion; use aws_types::config::Config; use aws_types::credentials::{ProvideCredentials, SharedCredentialsProvider}; + use smithy_types::retry::RetryConfig; /// Load a cross-service [`Config`](aws_types::config::Config) from the environment /// @@ -121,6 +122,7 @@ mod loader { #[derive(Default, Debug)] pub struct ConfigLoader { region: Option>, + retry_config: Option, credentials_provider: Option, } @@ -141,6 +143,22 @@ mod loader { self } + /// Override the retry_config used to build [`Config`](aws_types::config::Config). + /// + /// # Examples + /// ```rust + /// # use smithy_types::retry::RetryConfig; + /// # async fn create_config() { + /// let config = aws_config::from_env() + /// .retry_config(RetryConfig::new().with_max_attempts(2)) + /// .load().await; + /// # } + /// ``` + pub fn retry_config(mut self, retry_config: RetryConfig) -> Self { + self.retry_config = Some(retry_config); + self + } + /// Override the credentials provider used to build [`Config`](aws_types::config::Config). /// # Examples /// Override the credentials provider but load the default value for region: @@ -175,6 +193,13 @@ mod loader { } else { region::default_provider().region().await }; + + let retry_config = if let Some(retry_config) = self.retry_config { + retry_config + } else { + retry_config::default_provider().retry_config().await + }; + let credentials_provider = if let Some(provider) = self.credentials_provider { provider } else { @@ -182,8 +207,10 @@ mod loader { builder.set_region(region.clone()); SharedCredentialsProvider::new(builder.build().await) }; + Config::builder() .region(region) + .retry_config(retry_config) .credentials_provider(credentials_provider) .build() } diff --git a/aws/rust-runtime/aws-config/src/profile/mod.rs b/aws/rust-runtime/aws-config/src/profile/mod.rs index 7f1bbb59764c9eed949dbdbb0a8dcf5d3bbdfbbd..2549db4fa3ecda768e55a6a20e5b951bb50b7dae 100644 --- a/aws/rust-runtime/aws-config/src/profile/mod.rs +++ b/aws/rust-runtime/aws-config/src/profile/mod.rs @@ -14,6 +14,7 @@ pub use parser::{load, Profile, ProfileParseError, ProfileSet, Property}; pub mod credentials; pub mod region; +pub mod retry_config; #[doc(inline)] pub use credentials::ProfileFileCredentialsProvider; diff --git a/aws/rust-runtime/aws-config/src/profile/retry_config.rs b/aws/rust-runtime/aws-config/src/profile/retry_config.rs new file mode 100644 index 0000000000000000000000000000000000000000..c535734729271641d74f55b64ef8978f9cbc2d6d --- /dev/null +++ b/aws/rust-runtime/aws-config/src/profile/retry_config.rs @@ -0,0 +1,153 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +//! Load retry configuration properties from an AWS profile + +use std::str::FromStr; + +use aws_types::os_shim_internal::{Env, Fs}; +use smithy_types::retry::{RetryConfigBuilder, RetryConfigErr, RetryMode}; + +use crate::provider_config::ProviderConfig; + +/// Load retry configuration properties from a profile file +/// +/// This provider will attempt to load AWS shared configuration, then read retry configuration properties +/// from the active profile. +/// +/// # Examples +/// +/// **Loads 2 as the `max_attempts` to make when sending a request +/// ```ini +/// [default] +/// max_attempts = 2 +/// ``` +/// +/// **Loads `standard` as the `retry_mode` _if and only if_ the `other` profile is selected. +/// +/// ```ini +/// [profile other] +/// retry_mode = standard +/// ``` +/// +/// This provider is part of the [default retry_config provider chain](crate::default_provider::retry_config). +#[derive(Debug, Default)] +pub struct ProfileFileRetryConfigProvider { + fs: Fs, + env: Env, + profile_override: Option, +} + +/// Builder for [ProfileFileRetryConfigProvider] +#[derive(Default)] +pub struct Builder { + config: Option, + profile_override: Option, +} + +impl Builder { + /// Override the configuration for this provider + pub fn configure(mut self, config: &ProviderConfig) -> Self { + self.config = Some(config.clone()); + self + } + + /// Override the profile name used by the [ProfileFileRetryConfigProvider] + pub fn profile_name(mut self, profile_name: impl Into) -> Self { + self.profile_override = Some(profile_name.into()); + self + } + + /// Build a [ProfileFileRetryConfigProvider] from this builder + pub fn build(self) -> ProfileFileRetryConfigProvider { + let conf = self.config.unwrap_or_default(); + ProfileFileRetryConfigProvider { + env: conf.env(), + fs: conf.fs(), + profile_override: self.profile_override, + } + } +} + +impl ProfileFileRetryConfigProvider { + /// Create a new [ProfileFileRetryConfigProvider] + /// + /// To override the selected profile, set the `AWS_PROFILE` environment variable or use the [Builder]. + pub fn new() -> Self { + Self { + fs: Fs::real(), + env: Env::real(), + profile_override: None, + } + } + + /// [Builder] to construct a [ProfileFileRetryConfigProvider] + pub fn builder() -> Builder { + Builder::default() + } + + /// Attempt to create a new RetryConfigBuilder from a profile file. + pub async fn retry_config_builder(&self) -> Result { + let profile = match super::parser::load(&self.fs, &self.env).await { + Ok(profile) => profile, + Err(err) => { + tracing::warn!(err = %err, "failed to parse profile"); + // return an empty builder + return Ok(RetryConfigBuilder::new()); + } + }; + + let selected_profile = self + .profile_override + .as_deref() + .unwrap_or_else(|| profile.selected_profile()); + let selected_profile = match profile.get_profile(selected_profile) { + Some(profile) => profile, + None => { + tracing::warn!("failed to get selected '{}' profile", selected_profile); + // return an empty builder + return Ok(RetryConfigBuilder::new()); + } + }; + + let max_attempts = match selected_profile.get("max_attempts") { + Some(max_attempts) => match max_attempts.parse::() { + Ok(max_attempts) if max_attempts == 0 => { + return Err(RetryConfigErr::MaxAttemptsMustNotBeZero { + set_by: "aws profile".into(), + }); + } + Ok(max_attempts) => Some(max_attempts), + Err(source) => { + return Err(RetryConfigErr::FailedToParseMaxAttempts { + set_by: "aws profile".into(), + source, + }); + } + }, + None => None, + }; + + let retry_mode = match selected_profile.get("retry_mode") { + Some(retry_mode) => match RetryMode::from_str(&retry_mode) { + Ok(retry_mode) => Some(retry_mode), + Err(retry_mode_err) => { + return Err(RetryConfigErr::InvalidRetryMode { + set_by: "aws profile".into(), + source: retry_mode_err, + }); + } + }, + None => None, + }; + + let mut retry_config_builder = RetryConfigBuilder::new(); + retry_config_builder + .set_max_attempts(max_attempts) + .set_mode(retry_mode); + + Ok(retry_config_builder) + } +} diff --git a/aws/rust-runtime/aws-hyper/tests/e2e_test.rs b/aws/rust-runtime/aws-hyper/tests/e2e_test.rs index d5a050aeb538fe118278df6eb3404857bdcd1247..834ca3a7ee8b95b03289bdd25ae8567ce7a626cc 100644 --- a/aws/rust-runtime/aws-hyper/tests/e2e_test.rs +++ b/aws/rust-runtime/aws-hyper/tests/e2e_test.rs @@ -3,6 +3,18 @@ * SPDX-License-Identifier: Apache-2.0. */ +use std::convert::Infallible; +use std::error::Error; +use std::fmt; +use std::fmt::{Display, Formatter}; +use std::sync::Arc; +use std::time::{Duration, UNIX_EPOCH}; + +use bytes::Bytes; +use http::header::{AUTHORIZATION, USER_AGENT}; +use http::{self, Uri}; +use tokio::time::Instant; + use aws_endpoint::partition::endpoint::{Protocol, SignatureVersion}; use aws_endpoint::set_endpoint_resolver; use aws_http::user_agent::AwsUserAgent; @@ -13,9 +25,6 @@ use aws_types::credentials::SharedCredentialsProvider; use aws_types::region::Region; use aws_types::Credentials; use aws_types::SigningService; -use bytes::Bytes; -use http::header::{AUTHORIZATION, USER_AGENT}; -use http::{self, Uri}; use smithy_client::test_connection::TestConnection; use smithy_http::body::SdkBody; use smithy_http::operation; @@ -23,13 +32,6 @@ use smithy_http::operation::Operation; use smithy_http::response::ParseHttpResponse; use smithy_http::result::SdkError; use smithy_types::retry::{ErrorKind, ProvideErrorKind}; -use std::convert::Infallible; -use std::error::Error; -use std::fmt; -use std::fmt::{Display, Formatter}; -use std::sync::Arc; -use std::time::{Duration, UNIX_EPOCH}; -use tokio::time::Instant; #[derive(Clone)] struct TestOperationParser; @@ -160,7 +162,7 @@ async fn retry_test() { .body("response body") .unwrap() } - // 1 failing response followed by 1 succesful response + // 1 failing response followed by 1 successful response let events = vec![ // First operation (req(), err()), @@ -174,12 +176,11 @@ async fn retry_test() { (req(), err()), (req(), err()), (req(), err()), - (req(), err()), - (req(), err()), - (req(), err()), ]; let conn = TestConnection::new(events); - let retry_config = RetryConfig::default().with_base(|| 1_f64); + let retry_config = RetryConfig::default() + .with_max_attempts(4) + .with_base(|| 1_f64); let client = Client::new(conn.clone()).with_retry_config(retry_config); tokio::time::pause(); let initial = tokio::time::Instant::now(); @@ -204,10 +205,10 @@ async fn retry_test() { .call(test_operation()) .await .expect_err("all responses failed"); - // three more tries followed by failure - assert_eq!(conn.requests().len(), 8); + // 4 more tries followed by failure + assert_eq!(conn.requests().len(), 9); assert!(matches!(err, SdkError::ServiceError { .. })); - assert_time_passed(initial, Duration::from_secs(3)); + assert_time_passed(initial, Duration::from_secs(7)); } /// Validate that time has passed with a 5ms tolerance diff --git a/aws/rust-runtime/aws-types/Cargo.toml b/aws/rust-runtime/aws-types/Cargo.toml index 61121b0347294c29d349567aeffee67504367909..6523a5b2aac65de5a255e14a9325b88d8c73cd55 100644 --- a/aws/rust-runtime/aws-types/Cargo.toml +++ b/aws/rust-runtime/aws-types/Cargo.toml @@ -10,6 +10,7 @@ license = "Apache-2.0" [dependencies] tracing = "0.1" smithy-async = { path = "../../../rust-runtime/smithy-async" } +smithy-types = { path = "../../../rust-runtime/smithy-types" } zeroize = "1.4.1" [dev-dependencies] diff --git a/aws/rust-runtime/aws-types/src/config.rs b/aws/rust-runtime/aws-types/src/config.rs index 002f725a01c98248ceff466626930db4263503b8..84db88228537f217fdf4add4695b2ff0c3cb0e8b 100644 --- a/aws/rust-runtime/aws-types/src/config.rs +++ b/aws/rust-runtime/aws-types/src/config.rs @@ -9,12 +9,15 @@ //! //! This module contains an shared configuration representation that is agnostic from a specific service. +use smithy_types::retry::RetryConfig; + use crate::credentials::SharedCredentialsProvider; use crate::region::Region; /// AWS Shared Configuration pub struct Config { region: Option, + retry_config: Option, credentials_provider: Option, } @@ -22,6 +25,7 @@ pub struct Config { #[derive(Default)] pub struct Builder { region: Option, + retry_config: Option, credentials_provider: Option, } @@ -60,6 +64,42 @@ impl Builder { self } + /// Set the retry_config for the builder + /// + /// # Examples + /// ```rust + /// use aws_types::config::Config; + /// use smithy_types::retry::RetryConfig; + /// + /// let retry_config = RetryConfig::new().with_max_attempts(5); + /// let config = Config::builder().retry_config(retry_config).build(); + /// ``` + pub fn retry_config(mut self, retry_config: RetryConfig) -> Self { + self.set_retry_config(Some(retry_config)); + self + } + + /// Set the retry_config for the builder + /// + /// # Examples + /// ```rust + /// use aws_types::config::{Config, Builder}; + /// use smithy_types::retry::RetryConfig; + /// + /// fn disable_retries(builder: &mut Builder) { + /// let retry_config = RetryConfig::new().with_max_attempts(1); + /// builder.set_retry_config(Some(retry_config)); + /// } + /// + /// let mut builder = Config::builder(); + /// disable_retries(&mut builder); + /// let config = builder.build(); + /// ``` + pub fn set_retry_config(&mut self, retry_config: Option) -> &mut Self { + self.retry_config = retry_config; + self + } + /// Set the credentials provider for the builder /// /// # Examples @@ -116,6 +156,7 @@ impl Builder { pub fn build(self) -> Config { Config { region: self.region, + retry_config: self.retry_config, credentials_provider: self.credentials_provider, } } @@ -127,6 +168,11 @@ impl Config { self.region.as_ref() } + /// Configured retry config + pub fn retry_config(&self) -> Option<&RetryConfig> { + self.retry_config.as_ref() + } + /// Configured credentials provider pub fn credentials_provider(&self) -> Option<&SharedCredentialsProvider> { self.credentials_provider.as_ref() diff --git a/aws/rust-runtime/aws-types/src/os_shim_internal.rs b/aws/rust-runtime/aws-types/src/os_shim_internal.rs index 8bb19554a796e413431367724934761cd40e45f3..eb556d8eb8f1309a50b70c4cc48d979ce6958be6 100644 --- a/aws/rust-runtime/aws-types/src/os_shim_internal.rs +++ b/aws/rust-runtime/aws-types/src/os_shim_internal.rs @@ -7,8 +7,6 @@ //! - Reading environment variables //! - Reading from the file system -use crate::os_shim_internal::fs::Fake; -use crate::os_shim_internal::time_source::Inner; use std::collections::HashMap; use std::env::VarError; use std::ffi::OsString; @@ -18,6 +16,9 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use std::time::{Duration, SystemTime}; +use crate::os_shim_internal::fs::Fake; +use crate::os_shim_internal::time_source::Inner; + /// File system abstraction /// /// Simple abstraction enabling in-memory mocking of the file system @@ -93,6 +94,31 @@ impl Fs { }))) } + /// Create a fake process environment from a slice of tuples. + /// + /// # Examples + /// ```rust + /// # async fn example() { + /// use aws_types::os_shim_internal::Fs; + /// let mock_fs = Fs::from_slice(&[ + /// ("config", "[default]\nretry_mode = \"standard\""), + /// ]); + /// assert_eq!(mock_fs.read_to_end("config").await.unwrap(), b"[default]\nretry_mode = \"standard\""); + /// # } + /// ``` + pub fn from_slice<'a>(files: &[(&'a str, &'a str)]) -> Self { + let fs: HashMap> = files + .iter() + .map(|(k, v)| { + let k = (*k).to_owned(); + let v = v.as_bytes().to_vec(); + (k, v) + }) + .collect(); + + Self::from_map(fs) + } + /// Read the entire contents of a file /// /// **Note**: This function is currently `async` primarily for forward compatibility. Currently, @@ -310,11 +336,13 @@ mod time_source { #[cfg(test)] mod test { - use crate::os_shim_internal::{Env, Fs, ManualTimeSource, TimeSource}; - use futures_util::FutureExt; use std::env::VarError; use std::time::{Duration, UNIX_EPOCH}; + use futures_util::FutureExt; + + use crate::os_shim_internal::{Env, Fs, ManualTimeSource, TimeSource}; + #[test] fn env_works() { let env = Env::from_slice(&[("FOO", "BAR")]); diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt index 9ff9b58898dbd2c67f51527323e480852cc4e912..6a51580178c6798fffd8c8f53a361155e9e92fff 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt @@ -5,6 +5,7 @@ package software.amazon.smithy.rustsdk +import software.amazon.smithy.rust.codegen.smithy.RetryConfigDecorator import software.amazon.smithy.rust.codegen.smithy.customize.CombinedCodegenDecorator import software.amazon.smithy.rustsdk.customize.apigateway.ApiGatewayDecorator import software.amazon.smithy.rustsdk.customize.auth.DisabledAuthDecorator @@ -25,6 +26,9 @@ val DECORATORS = listOf( SharedConfigDecorator(), AwsPresigningDecorator(), + // Smithy specific decorators + RetryConfigDecorator(), + // Service specific decorators DisabledAuthDecorator(), ApiGatewayDecorator(), diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt index d979bf7cc75d9303a2145a6e802783f8d37196a8..002e75f21e456632f8a9c48a4eed62cc64ca96f2 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt @@ -93,7 +93,8 @@ private class AwsFluentClientExtensions(private val types: Types) { rustTemplate( """ pub fn from_conf_conn(conf: crate::Config, conn: C) -> Self { - let client = #{aws_hyper}::Client::new(conn); + let retry_config = conf.retry_config.as_ref().cloned().unwrap_or_default(); + let client = #{aws_hyper}::Client::new(conn).with_retry_config(retry_config.into()); Self { handle: std::sync::Arc::new(Handle { client, conf }) } } """, @@ -110,7 +111,8 @@ private class AwsFluentClientExtensions(private val types: Types) { ##[cfg(any(feature = "rustls", feature = "native-tls"))] pub fn from_conf(conf: crate::Config) -> Self { - let client = #{aws_hyper}::Client::https(); + let retry_config = conf.retry_config.as_ref().cloned().unwrap_or_default(); + let client = #{aws_hyper}::Client::https().with_retry_config(retry_config.into()); Self { handle: std::sync::Arc::new(Handle { client, conf }) } } """, diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt index 854f5019382945c76c8eb32a45e52d3c2cab8366..1d48e125c2d1d3df3304354bccd2418d0dce434b 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt @@ -24,27 +24,49 @@ import software.amazon.smithy.rust.codegen.smithy.generators.config.ServiceConfi /* Example Generated Code */ /* pub struct Config { - pub region: Option<::aws_types::region::Region>, + pub(crate) region: Option, } + +impl std::fmt::Debug for Config { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut config = f.debug_struct("Config"); + config.finish() + } +} + +impl Config { + pub fn builder() -> Builder { + Builder::default() + } +} + #[derive(Default)] pub struct Builder { - region: Option<::aws_types::region::Region>, + region: Option, } + impl Builder { - pub fn region(mut self, region_provider: impl ::aws_types::region::ProvideRegion) -> Self { - self.region = region_provider.region(); + pub fn new() -> Self { + Self::default() + } + + pub fn region(mut self, region: impl Into>) -> Self { + self.region = region.into(); self } pub fn build(self) -> Config { Config { - region: { - use ::aws_types::region::ProvideRegion; - self.region - .or_else(|| ::aws_types::region::default_provider().region()) - }, + region: self.region, + } } } + +#[test] +fn test_1() { + fn assert_send_sync() {} + assert_send_sync::(); +} */ class RegionDecorator : RustCodegenDecorator { diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SharedConfigDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SharedConfigDecorator.kt index 37d6d4310554b51f9c7a5f2566105b01d2a70174..a49c2f8a66981e174de3e2725f011edb7d2e4bb1 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SharedConfigDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SharedConfigDecorator.kt @@ -46,6 +46,7 @@ class SharedConfigDecorator : RustCodegenDecorator { fn from(input: &#{Config}) -> Self { let mut builder = Builder::default(); builder = builder.region(input.region().cloned()); + builder.set_retry_config(input.retry_config().cloned()); builder.set_credentials_provider(input.credentials_provider().cloned()); builder } diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointConfigCustomizationTest.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointConfigCustomizationTest.kt index aa69cbd4f5c8c331c5f0d8b298f3063ac726b9f8..53f15cc66e155226f1336984390d3f162ea24280 100644 --- a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointConfigCustomizationTest.kt +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/EndpointConfigCustomizationTest.kt @@ -8,12 +8,7 @@ package software.amazon.smithy.rustsdk import org.junit.jupiter.api.Test import software.amazon.smithy.model.node.ObjectNode import software.amazon.smithy.rust.codegen.rustlang.CargoDependency -import software.amazon.smithy.rust.codegen.testutil.asSmithyModel -import software.amazon.smithy.rust.codegen.testutil.compileAndTest -import software.amazon.smithy.rust.codegen.testutil.stubConfigProject -import software.amazon.smithy.rust.codegen.testutil.testCodegenContext -import software.amazon.smithy.rust.codegen.testutil.unitTest -import software.amazon.smithy.rust.codegen.testutil.validateConfigCustomizations +import software.amazon.smithy.rust.codegen.testutil.* import software.amazon.smithy.rust.codegen.util.lookup internal class EndpointConfigCustomizationTest { @@ -116,9 +111,9 @@ internal class EndpointConfigCustomizationTest { } @Test - fun `support region-based endpoint overrides`() { + fun `support region-specific endpoint overrides`() { val project = - stubConfigProject(endpointCustomization("test#TestService")) + stubConfigProject(endpointCustomization("test#TestService"), TestWorkspace.testProject()) project.lib { it.addDependency(awsTypes(AwsTestRuntimeConfig)) it.addDependency(CargoDependency.Http) @@ -139,9 +134,9 @@ internal class EndpointConfigCustomizationTest { } @Test - fun `support non-regionalized services`() { + fun `support region-agnostic services`() { val project = - stubConfigProject(endpointCustomization("test#NoRegions")) + stubConfigProject(endpointCustomization("test#NoRegions"), TestWorkspace.testProject()) project.lib { it.addDependency(awsTypes(AwsTestRuntimeConfig)) it.addDependency(CargoDependency.Http) diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/SigV4SigningCustomizationTest.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/SigV4SigningCustomizationTest.kt index e6b2e039f863b89dd61944be8bb2b2285ef8aae3..a5f75f9cc6486d42f5b1ff1d1182e26397d32d0d 100644 --- a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/SigV4SigningCustomizationTest.kt +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/SigV4SigningCustomizationTest.kt @@ -7,6 +7,7 @@ package software.amazon.smithy.rustsdk import org.junit.jupiter.api.Test import software.amazon.smithy.aws.traits.auth.SigV4Trait +import software.amazon.smithy.rust.codegen.testutil.TestWorkspace import software.amazon.smithy.rust.codegen.testutil.compileAndTest import software.amazon.smithy.rust.codegen.testutil.stubConfigProject import software.amazon.smithy.rust.codegen.testutil.unitTest @@ -19,7 +20,8 @@ internal class SigV4SigningCustomizationTest { AwsTestRuntimeConfig, true, SigV4Trait.builder().name("test-service").build() - ) + ), + TestWorkspace.testProject() ) project.lib { it.unitTest( diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RetryConfigDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RetryConfigDecorator.kt new file mode 100644 index 0000000000000000000000000000000000000000..472185f2c1bd9f58bf6fba47e49fa70b2618c7cd --- /dev/null +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RetryConfigDecorator.kt @@ -0,0 +1,160 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.rust.codegen.smithy + +import software.amazon.smithy.rust.codegen.rustlang.Writable +import software.amazon.smithy.rust.codegen.rustlang.rust +import software.amazon.smithy.rust.codegen.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.rustlang.writable +import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator +import software.amazon.smithy.rust.codegen.smithy.generators.LibRsCustomization +import software.amazon.smithy.rust.codegen.smithy.generators.LibRsSection +import software.amazon.smithy.rust.codegen.smithy.generators.config.ConfigCustomization +import software.amazon.smithy.rust.codegen.smithy.generators.config.ServiceConfig + +/* Example Generated Code */ +/* +pub struct Config { + pub(crate) retry_config: Option, +} +impl std::fmt::Debug for Config { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut config = f.debug_struct("Config"); + config.finish() + } +} +impl Config { + pub fn builder() -> Builder { + Builder::default() + } +} +#[derive(Default)] +pub struct Builder { + retry_config: Option, +} +impl Builder { + pub fn new() -> Self { + Self::default() + } + pub fn retry_config(mut self, retry_config: smithy_types::retry::RetryConfig) -> Self { + self.set_retry_config(Some(retry_config)); + self + } + pub fn set_retry_config( + &mut self, + retry_config: Option, + ) -> &mut Self { + self.retry_config = retry_config; + self + } + pub fn build(self) -> Config { + Config { + retry_config: self.retry_config, + } + } +} +#[test] +fn test_1() { + fn assert_send_sync() {} + assert_send_sync::(); +} + + */ + +class RetryConfigDecorator : RustCodegenDecorator { + override val name: String = "RetryConfig" + override val order: Byte = 0 + + override fun configCustomizations( + codegenContext: CodegenContext, + baseCustomizations: List + ): List { + return baseCustomizations + RetryConfigProviderConfig(codegenContext) + } + + override fun libRsCustomizations( + codegenContext: CodegenContext, + baseCustomizations: List + ): List { + return baseCustomizations + PubUseRetryConfig(codegenContext.runtimeConfig) + } +} + +class RetryConfigProviderConfig(codegenContext: CodegenContext) : ConfigCustomization() { + private val retryConfig = smithyTypesRetry(codegenContext.runtimeConfig) + private val moduleName = codegenContext.moduleName + private val moduleUseName = moduleName.replace("-", "_") + private val codegenScope = arrayOf("RetryConfig" to retryConfig.member("RetryConfig")) + override fun section(section: ServiceConfig) = writable { + when (section) { + is ServiceConfig.ConfigStruct -> rustTemplate( + "pub(crate) retry_config: Option<#{RetryConfig}>,", + *codegenScope + ) + is ServiceConfig.ConfigImpl -> emptySection + is ServiceConfig.BuilderStruct -> + rustTemplate("retry_config: Option<#{RetryConfig}>,", *codegenScope) + ServiceConfig.BuilderImpl -> + rustTemplate( + """ + /// Set the retry_config for the builder + /// + /// ## Examples + /// ```rust + /// use $moduleUseName::config::Config; + /// use #{RetryConfig}; + /// + /// let retry_config = RetryConfig::new().with_max_attempts(5); + /// let config = Config::builder().retry_config(retry_config).build(); + /// ``` + pub fn retry_config(mut self, retry_config: #{RetryConfig}) -> Self { + self.set_retry_config(Some(retry_config)); + self + } + + /// Set the retry_config for the builder + /// + /// ## Examples + /// ```rust + /// use $moduleUseName::config::{Builder, Config}; + /// use #{RetryConfig}; + /// + /// fn disable_retries(builder: &mut Builder) { + /// let retry_config = RetryConfig::new().with_max_attempts(1); + /// builder.set_retry_config(Some(retry_config)); + /// } + /// + /// let mut builder = Config::builder(); + /// disable_retries(&mut builder); + /// let config = builder.build(); + /// ``` + pub fn set_retry_config(&mut self, retry_config: Option<#{RetryConfig}>) -> &mut Self { + self.retry_config = retry_config; + self + } + """, + *codegenScope + ) + ServiceConfig.BuilderBuild -> rustTemplate( + """retry_config: self.retry_config,""", + *codegenScope + ) + } + } +} + +class PubUseRetryConfig(private val runtimeConfig: RuntimeConfig) : LibRsCustomization() { + override fun section(section: LibRsSection): Writable { + return when (section) { + is LibRsSection.Body -> writable { rust("pub use #T::RetryConfig;", smithyTypesRetry(runtimeConfig)) } + else -> emptySection + } + } +} + +// Generate path to the retry module in smithy_types +fun smithyTypesRetry(runtimeConfig: RuntimeConfig) = + RuntimeType("retry", runtimeConfig.runtimeCrate("types"), "smithy_types") diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/Rust.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/Rust.kt index 36764a8fe0b55c9026693c3efd59f02c57b87ade..0f41b342fd4a0073f038cd029b9173279b669a73 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/Rust.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/Rust.kt @@ -88,6 +88,7 @@ object TestWorkspace { } } + @Suppress("NAME_SHADOWING") fun testProject(symbolProvider: RustSymbolProvider? = null): TestWriterDelegator { val subprojectDir = subproject() val symbolProvider = symbolProvider ?: object : RustSymbolProvider { @@ -170,16 +171,7 @@ fun TestWriterDelegator.compileAndTest(runClippy: Boolean = false) { } """.asSmithyModel() this.finalize( - RustSettings( - ShapeId.from("fake#Fake"), - "test_${baseDir.toFile().nameWithoutExtension}", - "0.0.1", - moduleAuthors = listOf("test@module.com"), - runtimeConfig = TestRuntimeConfig, - codegenConfig = CodegenConfig(), - license = null, - model = stubModel - ), + rustSettings(stubModel), libRsCustomizations = listOf(), ) try { @@ -193,6 +185,18 @@ fun TestWriterDelegator.compileAndTest(runClippy: Boolean = false) { } } +fun TestWriterDelegator.rustSettings(stubModel: Model) = + RustSettings( + ShapeId.from("fake#Fake"), + "test_${baseDir.toFile().nameWithoutExtension}", + "0.0.1", + moduleAuthors = listOf("test@module.com"), + runtimeConfig = TestRuntimeConfig, + codegenConfig = CodegenConfig(), + license = null, + model = stubModel + ) + // TODO: unify these test helpers a bit fun String.shouldParseAsRust() { // quick hack via rustfmt @@ -239,6 +243,7 @@ fun RustWriter.compileAndTest( } } +@JvmOverloads private fun String.intoCrate( deps: Set, module: String? = null, diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/TestConfigCustomization.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/TestConfigCustomization.kt index f6f0d0c25895a5dded5820dbadc34bb4bbb13e9f..33ab7afde7579af08f5aae89d0b22cb40c5dadd9 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/TestConfigCustomization.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/TestConfigCustomization.kt @@ -43,16 +43,20 @@ fun stubCustomization(name: String): ConfigCustomization { * This test is not comprehensive, but it ensures that your customization generates Rust code that compiles and correctly * composes with other customizations. * */ -fun validateConfigCustomizations(vararg customization: ConfigCustomization): TestWriterDelegator { - val project = stubConfigProject(*customization) +@Suppress("NAME_SHADOWING") +fun validateConfigCustomizations( + customization: ConfigCustomization, + project: TestWriterDelegator? = null +): TestWriterDelegator { + val project = project ?: TestWorkspace.testProject() + stubConfigProject(customization, project) project.compileAndTest() return project } -fun stubConfigProject(vararg customization: ConfigCustomization): TestWriterDelegator { - val customizations = listOf(stubCustomization("a")) + customization.toList() + stubCustomization("b") +fun stubConfigProject(customization: ConfigCustomization, project: TestWriterDelegator): TestWriterDelegator { + val customizations = listOf(stubCustomization("a")) + customization + stubCustomization("b") val generator = ServiceConfigGenerator(customizations = customizations.toList()) - val project = TestWorkspace.testProject() project.withModule(RustModule.Config) { generator.render(it) it.unitTest( diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/TestHelpers.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/TestHelpers.kt index d49f499eb24eadd0770895939abd3907f7474498..02ff12ce7ef2c86df667b81a61c4038552dfc5f4 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/TestHelpers.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/TestHelpers.kt @@ -61,7 +61,7 @@ fun testCodegenContext( TestRuntimeConfig, serviceShape ?: ServiceShape.builder().version("test").id("test#Service").build(), ShapeId.from("test#Protocol"), - "test", + settings.moduleName, settings ) diff --git a/codegen/src/test/kotlin/software/amazon/smithy/rust/RetryConfigProviderConfigTest.kt b/codegen/src/test/kotlin/software/amazon/smithy/rust/RetryConfigProviderConfigTest.kt new file mode 100644 index 0000000000000000000000000000000000000000..812834cd785370a8f7803dd3bb0e8a8b49a64a7c --- /dev/null +++ b/codegen/src/test/kotlin/software/amazon/smithy/rust/RetryConfigProviderConfigTest.kt @@ -0,0 +1,43 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.rust + +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.smithy.RetryConfigProviderConfig +import software.amazon.smithy.rust.codegen.smithy.transformers.OperationNormalizer +import software.amazon.smithy.rust.codegen.smithy.transformers.RecursiveShapeBoxer +import software.amazon.smithy.rust.codegen.testutil.TestWorkspace +import software.amazon.smithy.rust.codegen.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.testutil.rustSettings +import software.amazon.smithy.rust.codegen.testutil.testCodegenContext +import software.amazon.smithy.rust.codegen.testutil.validateConfigCustomizations + +internal class RetryConfigProviderConfigTest { + private val baseModel = """ + namespace test + use aws.protocols#awsQuery + + structure SomeOutput { + @xmlAttribute + someAttribute: Long, + + someVal: String + } + + operation SomeOperation { + output: SomeOutput + } + """.asSmithyModel() + + @Test + fun `generates a valid config`() { + val model = RecursiveShapeBoxer.transform(OperationNormalizer.transform(baseModel)) + val project = TestWorkspace.testProject() + val codegenContext = testCodegenContext(model, settings = project.rustSettings(model)) + + validateConfigCustomizations(RetryConfigProviderConfig(codegenContext), project) + } +} diff --git a/rust-runtime/smithy-client/src/retry.rs b/rust-runtime/smithy-client/src/retry.rs index 50ef13478f32506b404cc7c016ec4f6ad67929d5..6c44a418562923097c2357a565a28942b7c4488c 100644 --- a/rust-runtime/smithy-client/src/retry.rs +++ b/rust-runtime/smithy-client/src/retry.rs @@ -51,7 +51,7 @@ pub struct Config { retry_cost: usize, no_retry_increment: usize, timeout_retry_cost: usize, - max_retries: u32, + max_attempts: u32, max_backoff: Duration, base: fn() -> f64, } @@ -70,9 +70,11 @@ impl Config { self } - /// Override the maximum number of retries - pub fn with_max_retries(mut self, max_retries: u32) -> Self { - self.max_retries = max_retries; + /// Override the maximum number of attempts + /// + /// `max_attempts` must be set to a value of at least `1` (indicating that retries are disabled). + pub fn with_max_attempts(mut self, max_attempts: u32) -> Self { + self.max_attempts = max_attempts; self } } @@ -84,7 +86,7 @@ impl Default for Config { retry_cost: RETRY_COST, no_retry_increment: 1, timeout_retry_cost: 10, - max_retries: MAX_RETRIES, + max_attempts: MAX_ATTEMPTS, max_backoff: Duration::from_secs(20), // by default, use a random base for exponential backoff base: fastrand::f64, @@ -92,7 +94,13 @@ impl Default for Config { } } -const MAX_RETRIES: u32 = 3; +impl From for Config { + fn from(conf: smithy_types::retry::RetryConfig) -> Self { + Self::default().with_max_attempts(conf.max_attempts()) + } +} + +const MAX_ATTEMPTS: u32 = 3; const INITIAL_RETRY_TOKENS: usize = 500; const RETRY_COST: usize = 5; @@ -145,12 +153,22 @@ impl Default for Standard { } } -#[derive(Default, Clone, Debug)] +#[derive(Clone, Debug)] struct RequestLocalRetryState { attempts: u32, last_quota_usage: Option, } +impl Default for RequestLocalRetryState { + fn default() -> Self { + Self { + // Starts at one to account for the initial request that failed and warranted a retry + attempts: 1, + last_quota_usage: None, + } + } +} + impl RequestLocalRetryState { pub fn new() -> Self { Self::default() @@ -236,7 +254,7 @@ impl RetryHandler { return None; } Err(e) => { - if self.local.attempts == self.config.max_retries - 1 { + if self.local.attempts == self.config.max_attempts { return None; } self.shared.quota_acquire(&e, &self.config)? @@ -250,7 +268,9 @@ impl RetryHandler { */ let r: i32 = 2; let b = (self.config.base)(); - let backoff = b * (r.pow(self.local.attempts) as f64); + // `self.local.attempts` tracks number of requests made including the initial request + // The initial attempt shouldn't count towards backoff calculations so we subtract it + let backoff = b * (r.pow(self.local.attempts - 1) as f64); let backoff = Duration::from_secs_f64(backoff).min(self.config.max_backoff); let next = RetryHandler { local: RequestLocalRetryState { @@ -380,7 +400,7 @@ mod test { #[test] fn backoff_timing() { let mut conf = test_config(); - conf.max_retries = 5; + conf.max_attempts = 5; let policy = Standard::new(conf).new_request_policy(); let (policy, dur) = policy .attempt_retry(Err(ErrorKind::ServerError)) @@ -414,7 +434,7 @@ mod test { #[test] fn max_backoff_time() { let mut conf = test_config(); - conf.max_retries = 5; + conf.max_attempts = 5; conf.max_backoff = Duration::from_secs(3); let policy = Standard::new(conf).new_request_policy(); let (policy, dur) = policy diff --git a/rust-runtime/smithy-types/src/retry.rs b/rust-runtime/smithy-types/src/retry.rs index be6a5cf74fa4f1b2bcb0991268f63805d8e00c4d..e96344b453818e5ced34ffdba36e9ada91495756 100644 --- a/rust-runtime/smithy-types/src/retry.rs +++ b/rust-runtime/smithy-types/src/retry.rs @@ -5,6 +5,10 @@ //! This module defines types that describe when to retry given a response. +use std::borrow::Cow; +use std::fmt::{Display, Formatter}; +use std::num::ParseIntError; +use std::str::FromStr; use std::time::Duration; #[derive(Clone, Copy, Eq, PartialEq, Debug)] @@ -68,3 +72,288 @@ pub enum RetryKind { /// The response associated with this variant should not be retried. NotRetryable, } + +#[non_exhaustive] +#[derive(Eq, PartialEq, Debug, Clone, Copy)] +pub enum RetryMode { + Standard, + Adaptive, +} + +const VALID_RETRY_MODES: &[RetryMode] = &[RetryMode::Standard]; + +#[derive(Debug)] +pub struct RetryModeParseErr(String); + +impl Display for RetryModeParseErr { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "error parsing string '{}' as RetryMode, valid options are: {:#?}", + self.0, VALID_RETRY_MODES + ) + } +} + +impl std::error::Error for RetryModeParseErr {} + +impl FromStr for RetryMode { + type Err = RetryModeParseErr; + + fn from_str(string: &str) -> Result { + let string = string.trim(); + // eq_ignore_ascii_case is OK here because the only strings we need to check for are ASCII + if string.eq_ignore_ascii_case("standard") { + Ok(RetryMode::Standard) + // TODO we can uncomment this once this issue is addressed: https://github.com/awslabs/aws-sdk-rust/issues/247 + // } else if string.eq_ignore_ascii_case("adaptive") { + // Ok(RetryMode::Adaptive) + } else { + Err(RetryModeParseErr(string.to_owned())) + } + } +} + +#[non_exhaustive] +#[derive(Debug, Default, Clone, PartialEq)] +pub struct RetryConfigBuilder { + pub mode: Option, + pub max_attempts: Option, +} + +impl RetryConfigBuilder { + pub fn new() -> Self { + Default::default() + } + + pub fn set_mode(&mut self, retry_mode: Option) -> &mut Self { + self.mode = retry_mode; + self + } + + pub fn set_max_attempts(&mut self, max_attempts: Option) -> &mut Self { + self.max_attempts = max_attempts; + self + } + + pub fn mode(mut self, mode: RetryMode) -> Self { + self.set_mode(Some(mode)); + self + } + + pub fn max_attempts(mut self, max_attempts: u32) -> Self { + self.set_max_attempts(Some(max_attempts)); + self + } + + /// Merge two builders together. Values from `other` will only be used as a fallback for values + /// from `self` Useful for merging configs from different sources together when you want to + /// handle "precedence" per value instead of at the config level + /// + /// # Example + /// + /// ```rust + /// # use smithy_types::retry::{RetryMode, RetryConfigBuilder}; + /// let a = RetryConfigBuilder::new().max_attempts(1); + /// let b = RetryConfigBuilder::new().max_attempts(5).mode(RetryMode::Adaptive); + /// let retry_config = a.merge_with(b).build(); + /// // A's value take precedence over B's value + /// assert_eq!(retry_config.max_attempts(), 1); + /// // A never set a retry mode so B's value was used + /// assert_eq!(retry_config.mode(), RetryMode::Adaptive); + /// ``` + pub fn merge_with(self, other: Self) -> Self { + Self { + mode: self.mode.or(other.mode), + max_attempts: self.max_attempts.or(other.max_attempts), + } + } + + pub fn build(self) -> RetryConfig { + RetryConfig { + mode: self.mode.unwrap_or(RetryMode::Standard), + max_attempts: self.max_attempts.unwrap_or(3), + } + } +} + +#[non_exhaustive] +#[derive(Debug, Clone, PartialEq)] +pub struct RetryConfig { + mode: RetryMode, + max_attempts: u32, +} + +impl RetryConfig { + pub fn new() -> Self { + Default::default() + } + + pub fn disabled() -> Self { + Self::default().with_max_attempts(1) + } + + pub fn with_retry_mode(mut self, retry_mode: RetryMode) -> Self { + self.mode = retry_mode; + self + } + + pub fn with_max_attempts(mut self, max_attempts: u32) -> Self { + self.max_attempts = max_attempts; + self + } + + pub fn mode(&self) -> RetryMode { + self.mode + } + + pub fn max_attempts(&self) -> u32 { + self.max_attempts + } +} + +impl Default for RetryConfig { + fn default() -> Self { + Self { + mode: RetryMode::Standard, + max_attempts: 3, + } + } +} + +#[derive(Debug)] +pub enum RetryConfigErr { + InvalidRetryMode { + source: RetryModeParseErr, + set_by: Cow<'static, str>, + }, + MaxAttemptsMustNotBeZero { + set_by: Cow<'static, str>, + }, + FailedToParseMaxAttempts { + source: ParseIntError, + set_by: Cow<'static, str>, + }, + AdaptiveModeIsNotSupported { + set_by: Cow<'static, str>, + }, +} + +impl Display for RetryConfigErr { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + use RetryConfigErr::*; + match self { + InvalidRetryMode { set_by, source } => { + write!(f, "invalid configuration set by {}: {}", set_by, source) + } + MaxAttemptsMustNotBeZero { set_by } => { + write!(f, "invalid configuration set by {}: It is invalid to set max attempts to 0. Unset it or set it to an integer greater than or equal to one.", set_by) + } + FailedToParseMaxAttempts { set_by, source } => { + write!( + f, + "failed to parse max attempts set by {}: {}", + set_by, source + ) + } + AdaptiveModeIsNotSupported { set_by } => { + write!(f, "invalid configuration set by {}: Setting retry mode to 'adaptive' is not yet supported. Unset it or set it to 'standard' mode.", set_by) + } + } + } +} + +impl std::error::Error for RetryConfigErr { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use RetryConfigErr::*; + match self { + InvalidRetryMode { source, .. } => Some(source), + FailedToParseMaxAttempts { source, .. } => Some(source), + _ => None, + } + } +} + +#[cfg(test)] +mod tests { + use crate::retry::{RetryConfigBuilder, RetryMode}; + use std::str::FromStr; + + #[test] + fn retry_config_builder_merge_with_favors_self_values_over_other_values() { + let self_builder = RetryConfigBuilder::new() + .max_attempts(1) + .mode(RetryMode::Adaptive); + let other_builder = RetryConfigBuilder::new() + .max_attempts(5) + .mode(RetryMode::Standard); + let retry_config = self_builder.merge_with(other_builder).build(); + + assert_eq!(retry_config.max_attempts, 1); + assert_eq!(retry_config.mode, RetryMode::Adaptive); + } + + #[test] + fn retry_mode_from_str_parses_valid_strings_regardless_of_casing() { + assert_eq!( + RetryMode::from_str("standard").ok(), + Some(RetryMode::Standard) + ); + assert_eq!( + RetryMode::from_str("STANDARD").ok(), + Some(RetryMode::Standard) + ); + assert_eq!( + RetryMode::from_str("StAnDaRd").ok(), + Some(RetryMode::Standard) + ); + // assert_eq!( + // RetryMode::from_str("adaptive").ok(), + // Some(RetryMode::Adaptive) + // ); + // assert_eq!( + // RetryMode::from_str("ADAPTIVE").ok(), + // Some(RetryMode::Adaptive) + // ); + // assert_eq!( + // RetryMode::from_str("aDaPtIvE").ok(), + // Some(RetryMode::Adaptive) + // ); + } + + #[test] + fn retry_mode_from_str_ignores_whitespace_before_and_after() { + assert_eq!( + RetryMode::from_str(" standard ").ok(), + Some(RetryMode::Standard) + ); + assert_eq!( + RetryMode::from_str(" STANDARD ").ok(), + Some(RetryMode::Standard) + ); + assert_eq!( + RetryMode::from_str(" StAnDaRd ").ok(), + Some(RetryMode::Standard) + ); + // assert_eq!( + // RetryMode::from_str(" adaptive ").ok(), + // Some(RetryMode::Adaptive) + // ); + // assert_eq!( + // RetryMode::from_str(" ADAPTIVE ").ok(), + // Some(RetryMode::Adaptive) + // ); + // assert_eq!( + // RetryMode::from_str(" aDaPtIvE ").ok(), + // Some(RetryMode::Adaptive) + // ); + } + + #[test] + fn retry_mode_from_str_wont_parse_invalid_strings() { + assert_eq!(RetryMode::from_str("std").ok(), None); + assert_eq!(RetryMode::from_str("aws").ok(), None); + assert_eq!(RetryMode::from_str("s t a n d a r d").ok(), None); + assert_eq!(RetryMode::from_str("a d a p t i v e").ok(), None); + } +}