Unverified Commit a9a691b6 authored by Zelda Hessler's avatar Zelda Hessler Committed by GitHub
Browse files

Feature: Fine-grained Timeout Configuration (#831)



* add: TimeoutConfig
add: provider to fetch timeout from profile
add: provider to fetch timeout from environment
add: default provider for TimeoutConfigs
update: aws_config::Config to support timeout conf
update: rustdoc lint

* add: TimeoutConfigBuilder::merge_with test

* update: changelogs

* add: timeout_config to client and builder
add: generic timeout service
add: non-working timeout layer

* add: timeout layer/service with configurable duration
add: test that ensures timeout service works

* fix: eliminate useless clones

* feature: Kotlin decorator for TimeoutConfig
add: tests for timeout-related codegen
update: incorrect package path in RetryConfigDecorator.kt
fix: outdated aws-config timeout config code

* remove: link to struct in external crate
fix: outdated doc test
fix: add missing import to doc test
fix: copypaste error in doc test
update: outdated lint name

* update: comment on Builder::timeout_config panics
add: test ensuring timeouts can't be infinite
update: use a floating point number in a timeout doc
update: message for failed profile load to mention that profile will be skipped
remove: commented out code
update: attempt to make difference between api_call_timeout and api_call_attempt_timeout clearer
update: outdated doc comments
update: TimeoutConfigError descriptions

* formatting: arrange imports in aws_smithy_client lib.rs
add: todo for improving timeout error categorization

* update: var parsers to work without needless allocations
update: improve example for default timeout config provider
update: improve example for default retry config provider
fix: unhelpful doc comment for TimeoutLayerFuture<T>
remove: outdated TODO

* fix: various typos in docs
add: PR links to changelogs
format: doc references to structs to look nicer
add: note about expected form and unit of timeout config data
update: expand ProfileFileTimeoutConfigProvider example

* fix: relocate provider config tests to the correct package/directory
remove: unimplemented timeouts from `Settings`

* add: S3 integration test for timeouts
update: re-enable TimeoutLayers
add: "list_of_set_timeouts" logging helper to TimeoutConfig
refactor: the way TimeoutService handles futures so that it can work better with no timeout set
add: helper structs to make creating timeout services easier

* update: split timeout_config example into multiple lines

* fix: Clippy lints

* fix: outdated test

* fix: more clippy lints

* fix: typo
add: TimeoutConfig example

* Update CHANGELOG.md

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>

* Update aws/rust-runtime/aws-config/src/default_provider.rs

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>

* Update aws/rust-runtime/aws-config/src/default_provider.rs

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>

* Update aws/rust-runtime/aws-config/src/default_provider.rs

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>

* Update aws/rust-runtime/aws-config/src/default_provider.rs

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>

* Update aws/rust-runtime/aws-config/src/default_provider.rs

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>

* Update aws/rust-runtime/aws-config/src/default_provider.rs

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>

* feature: user-configurable AsyncSleep impls
update: fallback to sleep impl that sleeps forever instead of sleep impl being optional

* Update rust-runtime/aws-smithy-types/src/timeout.rs

Co-authored-by: default avatarJohn DiSanti <jdisanti@amazon.com>

* update: changelogs
format: SleepImplDecorator.kt
update: TimeoutConfig doc
remove: list_of_set_timeouts in favor of Debug impl for TimeoutConfig

* Apply suggestions from code review

Co-authored-by: default avatarJohn DiSanti <jdisanti@amazon.com>

* fix: broken macro doc test by ignoring it
fix: outdated struct ref in doc
fix: outdated generated doc

* fix: mode broken doc tests

* fix: broken doc test

* attempt to fix CI-only doc test error

* add: moduleUseName method to CodegenContext
remove: pub use reexports from timeout and sleep impl decorators
add: pub use reexports to aws_config for timeout and retry configs
undo: default_sleep_impl changes
attempt to add tokio time feature to S3 integration test

* Update timeout.rs

* Apply suggestions from code review

Co-authored-by: default avatarJohn DiSanti <jdisanti@amazon.com>

* refactor: consolidate timeout parsing logic
rename: `RetryConfigBuilder::merge_with` to `RetryConfigBuilder::take_unset_from`
refactor: move timeout parsing tests to timeouts.rs
add: entry to SDK changelog noting the renaming
remove: redundant feature from s3 integration test Cargo.toml
update: various setters added by this PR to have the same form as our preexisting setters
add: extra info to the warning emitted when ConfigLoader calls default_async_sleep and gets None

* fix: tests broken when implementing suggestions

* update: doc hide sleep_impl for aws_types::Config
remove: leftover comment

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>
Co-authored-by: default avatarJohn DiSanti <jdisanti@amazon.com>
parent cab7de3f
Loading
Loading
Loading
Loading
+5 −1
Original line number Diff line number Diff line
@@ -7,13 +7,17 @@ vNext (Month Day, Year)
**Breaking Changes**
- (aws-smithy-client): Extraneous `pub use SdkSuccess` removed from `aws_smithy_client::hyper_ext`. (smithy-rs#855)

**New this week**

- Timeouts for requests are now configurable. You can set separate timeouts for each individual request attempt and all attempts made for a request. (smithy-rs#831)

v0.29.0-alpha (November 11th, 2021)
===================================

**Breaking Changes**

Several breaking changes around `aws_smithy_types::Instant` were introduced by smithy-rs#849:
- `aws_smithy_types::Instant` from was renamed to `DateTime` to avoid confusion with the standard library's monotonically nondecreasing `Instant` type.
- `aws_smithy_types::Instant` from was renamed to `DateTime` to avoid confusion with the standard library's monotonically non-decreasing `Instant` type.
- `DateParseError` in `aws_smithy_types` has been renamed to `DateTimeParseError` to match the type that's being parsed.
- The `chrono-conversions` feature and associated functions have been moved to the `aws-smithy-types-convert` crate.
  - Calls to `Instant::from_chrono` should be changed to:
+13 −7
Original line number Diff line number Diff line
vNext (Month Day, Year)
=======================

**New this release**
- Improve docs on `aws-smithy-client` (smithy-rs#855)
**TODO Upon release**
- Update README & aws-sdk-rust CI for MSRV upgrade to 1.54

**Breaking Changes**
- (aws-smithy-client): Extraneous `pub use SdkSuccess` removed from `aws_smithy_client::hyper_ext`. (smithy-rs#855)
**New this week**

- :tada: Timeouts for requests are now configurable. You can set a timeout for each individual request attempt or for all attempts made for a request. (smithy-rs#831)

**Breaking changes**

- `RetryConfigBuilder::merge_with` has been renamed to `RetryConfigBuilder::take_unset_from`

v0.0.26-alpha (TBD)
=======================
===================================
**New this release**
- Improve docs on `aws-smithy-client` (smithy-rs#855)

**TODO Upon release**
- Update README & aws-sdk-rust CI for MSRV upgrade to 1.54
**Breaking Changes**
- (aws-smithy-client): Extraneous `pub use SdkSuccess` removed from `aws_smithy_client::hyper_ext`. (smithy-rs#855)

**Breaking Changes**

+134 −6
Original line number Diff line number Diff line
@@ -104,15 +104,26 @@ pub mod retry_config {
    ///
    /// # Example
    ///
    /// When running [`aws_config::from_env()`](crate::from_env()), a [`ConfigLoader`](crate::ConfigLoader)
    /// is created that will then create a [`RetryConfig`] from the default_provider. There is no
    /// need to call `default_provider` and the example below is only for illustration purposes.
    ///
    /// ```no_run
    /// # use std::error::Error;
    /// # #[tokio::main]
    /// # async fn main() -> Result<(), Box<dyn Error>> {
    /// 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;
    ///
    /// // Load a retry config from a specific profile
    /// let retry_config = retry_config::default_provider()
    ///     .profile_name("other_profile")
    ///     .retry_config()
    ///     .await;
    /// let config = aws_config::from_env()
    ///     // Override the retry config set by the default profile
    ///     .retry_config(retry_config)
    ///     .load()
    ///     .await;
    /// // instantiate a service client:
    /// // <my_aws_service>::Client::new(&config);
    /// #     Ok(())
@@ -130,7 +141,6 @@ pub mod retry_config {
    }

    impl Builder {
        #[doc(hidden)]
        /// Configure the default chain
        ///
        /// Exposed for overriding the environment when unit-testing providers
@@ -172,7 +182,9 @@ pub mod retry_config {
                Err(err) => panic!("{}", err),
            };

            builder_from_env.merge_with(builder_from_profile).build()
            builder_from_env
                .take_unset_from(builder_from_profile)
                .build()
        }
    }
}
@@ -272,6 +284,122 @@ pub mod app_name {
    }
}

/// Default timeout configuration provider chain
pub mod timeout_config {
    use aws_smithy_types::timeout::TimeoutConfig;

    use crate::environment::timeout_config::EnvironmentVariableTimeoutConfigProvider;
    use crate::profile;
    use crate::provider_config::ProviderConfig;

    /// Default [`TimeoutConfig`] Provider chain
    ///
    /// Unlike other credentials and region, [`TimeoutConfig`] has no related `TimeoutConfigProvider` trait. Instead,
    /// a builder struct is returned which has a similar API.
    ///
    /// This provider will check the following sources in order:
    /// 1. [Environment variables](EnvironmentVariableTimeoutConfigProvider)
    /// 2. [Profile file](crate::profile::timeout_config::ProfileFileTimeoutConfigProvider) (`~/.aws/config`)
    ///
    /// # Example
    ///
    /// ```no_run
    /// # use std::error::Error;
    /// # #[tokio::main]
    /// # async fn main() {
    /// use aws_config::default_provider::timeout_config;
    ///
    /// // Load a timeout config from a specific profile
    /// let timeout_config = timeout_config::default_provider()
    ///     .profile_name("other_profile")
    ///     .timeout_config()
    ///     .await;
    /// let config = aws_config::from_env()
    ///     // Override the timeout config set by the default profile
    ///     .timeout_config(timeout_config)
    ///     .load()
    ///     .await;
    /// // instantiate a service client:
    /// // <my_aws_service>::Client::new(&config);
    /// # }
    /// ```
    pub fn default_provider() -> Builder {
        Builder::default()
    }

    /// Builder for [`TimeoutConfig`] that checks the environment variables and AWS profile files for configuration
    #[derive(Default)]
    pub struct Builder {
        env_provider: EnvironmentVariableTimeoutConfigProvider,
        profile_file: profile::timeout_config::Builder,
    }

    impl Builder {
        /// Configure the default chain
        ///
        /// Exposed for overriding the environment when unit-testing providers
        pub fn configure(mut self, configuration: &ProviderConfig) -> Self {
            self.env_provider =
                EnvironmentVariableTimeoutConfigProvider::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 [`TimeoutConfig`](aws_smithy_types::timeout::TimeoutConfig) from following sources in order:
        /// 1. [Environment variables](crate::environment::timeout_config::EnvironmentVariableTimeoutConfigProvider)
        /// 2. [Profile file](crate::profile::timeout_config::ProfileFileTimeoutConfigProvider)
        ///
        /// Precedence is considered on a per-field basis. If no timeout is specified, requests will never time out.
        ///
        /// # Panics
        ///
        /// This will panic if:
        /// - a timeout is set to `NaN`, a negative number, or infinity
        /// - a timeout can't be parsed as a floating point number
        pub async fn timeout_config(self) -> TimeoutConfig {
            // 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.timeout_config() {
                Ok(timeout_config_builder) => timeout_config_builder,
                Err(err) => panic!("{}", err),
            };
            let builder_from_profile = match self.profile_file.build().timeout_config().await {
                Ok(timeout_config_builder) => timeout_config_builder,
                Err(err) => panic!("{}", err),
            };

            let conf = builder_from_env.take_unset_from(builder_from_profile);

            if conf.tls_negotiation_timeout().is_some() {
                tracing::warn!(
                    "A TLS negotiation timeout was set but that feature is currently unimplemented so the setting will be ignored. \
                    To help us prioritize support for this feature, please upvote aws-sdk-rust#151 (https://github.com/awslabs/aws-sdk-rust/issues/151)")
            }

            if conf.connect_timeout().is_some() {
                tracing::warn!(
                    "A connect timeout was set but that feature is currently unimplemented so the setting will be ignored. \
                    To help us prioritize support for this feature, please upvote aws-sdk-rust#151 (https://github.com/awslabs/aws-sdk-rust/issues/151)")
            }

            if conf.read_timeout().is_some() {
                tracing::warn!(
                    "A read timeout was set but that feature is currently unimplemented so the setting will be ignored. \
                    To help us prioritize support for this feature, please upvote aws-sdk-rust#151 (https://github.com/awslabs/aws-sdk-rust/issues/151)")
            }

            conf
        }
    }
}

/// Default credentials provider chain
pub mod credentials {
    use std::borrow::Cow;
+5 −0
Original line number Diff line number Diff line
@@ -9,6 +9,7 @@ pub use app_name::EnvironmentVariableAppNameProvider;
/// Load credentials from the environment
pub mod credentials;
pub use credentials::EnvironmentVariableCredentialsProvider;

/// Load regions from the environment
pub mod region;
pub use region::EnvironmentVariableRegionProvider;
@@ -16,3 +17,7 @@ pub use region::EnvironmentVariableRegionProvider;
/// Load retry behavior configuration from the environment
pub mod retry_config;
pub use retry_config::EnvironmentVariableRetryConfigProvider;

/// Load timeout configuration from the environment
pub mod timeout_config;
pub use timeout_config::EnvironmentVariableTimeoutConfigProvider;
+2 −3
Original line number Diff line number Diff line
@@ -14,15 +14,14 @@ 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
/// in order to build a retry config.
#[derive(Debug, Default)]
pub struct EnvironmentVariableRetryConfigProvider {
    env: Env,
}

impl EnvironmentVariableRetryConfigProvider {
    /// Create a new `EnvironmentVariableRetryConfigProvider`
    /// Create a new [`EnvironmentVariableRetryConfigProvider`]
    pub fn new() -> Self {
        EnvironmentVariableRetryConfigProvider { env: Env::real() }
    }
Loading