Unverified Commit bcfc2112 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Simplify default config and add default async sleep (#3071)

While implementing identity caching, I noticed we don't have a default
async sleep impl wired up for generic clients, which was causing caching
to panic in many tests since it needs a sleep impl for timeouts. I
likely need to figure out what to do other than panic if there's no
sleep impl, but that's a problem for a different PR.

This PR adds a default sleep impl to generic clients, and also
simplifies how default config works. Previously, the generated config
`Builder::build` method set all the defaults with a series of "if not
set, then set default" statements. In this PR, defaults are registered
via default ordered runtime plugins.

Additionally, I cleaned up the standard retry strategy:
- The `TokenBucketPartition` didn't appear to be used at all, so I
deleted it.
- `StandardRetryStrategy` was taking retry config at construction, which
means it isn't possible to config override the retry config unless the
strategy is recreated. It now doesn't take any config at all during
construction.
- The adaptive retry client rate limiter was created at construction
based on retry config at that point in time. This means config overrides
wouldn't work with it, so it is also no longer set up at construction
time.
- Removed some unused runtime plugins.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent d48acae8
Loading
Loading
Loading
Loading
+4 −3
Original line number Diff line number Diff line
@@ -238,13 +238,14 @@ impl ImdsCommonRuntimePlugin {
    fn new(
        config: &ProviderConfig,
        endpoint_resolver: ImdsEndpointResolver,
        retry_config: &RetryConfig,
        retry_config: RetryConfig,
        timeout_config: TimeoutConfig,
    ) -> Self {
        let mut layer = Layer::new("ImdsCommonRuntimePlugin");
        layer.store_put(AuthSchemeOptionResolverParams::new(()));
        layer.store_put(EndpointResolverParams::new(()));
        layer.store_put(SensitiveOutput);
        layer.store_put(retry_config);
        layer.store_put(timeout_config);
        layer.store_put(user_agent());

@@ -255,7 +256,7 @@ impl ImdsCommonRuntimePlugin {
                .with_endpoint_resolver(Some(endpoint_resolver))
                .with_interceptor(UserAgentInterceptor::new())
                .with_retry_classifier(SharedRetryClassifier::new(ImdsResponseRetryClassifier))
                .with_retry_strategy(Some(StandardRetryStrategy::new(retry_config)))
                .with_retry_strategy(Some(StandardRetryStrategy::new()))
                .with_time_source(Some(config.time_source()))
                .with_sleep_impl(config.sleep_impl()),
        }
@@ -423,7 +424,7 @@ impl Builder {
        let common_plugin = SharedRuntimePlugin::new(ImdsCommonRuntimePlugin::new(
            &config,
            endpoint_resolver,
            &retry_config,
            retry_config,
            timeout_config,
        ));
        let operation = Operation::builder()
+1 −87
Original line number Diff line number Diff line
@@ -7,20 +7,16 @@ package software.amazon.smithy.rust.codegen.client.smithy.customizations

import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule
import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginCustomization
import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginSection
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigCustomization
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ServiceConfig
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.util.sdkId

class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenContext) : ConfigCustomization() {
class ResiliencyConfigCustomization(codegenContext: ClientCodegenContext) : ConfigCustomization() {
    private val runtimeConfig = codegenContext.runtimeConfig
    private val retryConfig = RuntimeType.smithyTypes(runtimeConfig).resolve("retry")
    private val sleepModule = RuntimeType.smithyAsync(runtimeConfig).resolve("rt::sleep")
@@ -44,8 +40,6 @@ class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenCon
        "StandardRetryStrategy" to retries.resolve("strategy::StandardRetryStrategy"),
        "SystemTime" to RuntimeType.std.resolve("time::SystemTime"),
        "TimeoutConfig" to timeoutModule.resolve("TimeoutConfig"),
        "TokenBucket" to retries.resolve("TokenBucket"),
        "TokenBucketPartition" to retries.resolve("TokenBucketPartition"),
    )

    override fun section(section: ServiceConfig) =
@@ -281,57 +275,6 @@ class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenCon
                    )
                }

                is ServiceConfig.BuilderBuild -> {
                    rustTemplate(
                        """
                        if layer.load::<#{RetryConfig}>().is_none() {
                            layer.store_put(#{RetryConfig}::disabled());
                        }
                        let retry_config = layer.load::<#{RetryConfig}>().expect("set to default above").clone();

                        if layer.load::<#{RetryPartition}>().is_none() {
                            layer.store_put(#{RetryPartition}::new("${codegenContext.serviceShape.sdkId()}"));
                        }
                        let retry_partition = layer.load::<#{RetryPartition}>().expect("set to default above").clone();

                        if retry_config.has_retry() {
                            #{debug}!("using retry strategy with partition '{}'", retry_partition);
                        }

                        if retry_config.mode() == #{RetryMode}::Adaptive {
                            if let #{Some}(time_source) = self.runtime_components.time_source() {
                                let seconds_since_unix_epoch = time_source
                                    .now()
                                    .duration_since(#{SystemTime}::UNIX_EPOCH)
                                    .expect("the present takes place after the UNIX_EPOCH")
                                    .as_secs_f64();
                                let client_rate_limiter_partition = #{ClientRateLimiterPartition}::new(retry_partition.clone());
                                let client_rate_limiter = CLIENT_RATE_LIMITER.get_or_init(client_rate_limiter_partition, || {
                                    #{ClientRateLimiter}::new(seconds_since_unix_epoch)
                                });
                                layer.store_put(client_rate_limiter);
                            }
                        }

                        // The token bucket is used for both standard AND adaptive retries.
                        let token_bucket_partition = #{TokenBucketPartition}::new(retry_partition);
                        let token_bucket = TOKEN_BUCKET.get_or_init(token_bucket_partition, #{TokenBucket}::default);
                        layer.store_put(token_bucket);

                        // TODO(enableNewSmithyRuntimeCleanup): Should not need to provide a default once smithy-rs##2770
                        //  is resolved
                        if layer.load::<#{TimeoutConfig}>().is_none() {
                            layer.store_put(#{TimeoutConfig}::disabled());
                        }

                        self.runtime_components.set_retry_strategy(#{Some}(
                            #{SharedRetryStrategy}::new(#{StandardRetryStrategy}::new(&retry_config)))
                        );
                        """,
                        *codegenScope,
                    )
                }

                else -> emptySection
            }
        }
@@ -366,32 +309,3 @@ class ResiliencyReExportCustomization(codegenContext: ClientCodegenContext) {
        }
    }
}

class ResiliencyServiceRuntimePluginCustomization(codegenContext: ClientCodegenContext) : ServiceRuntimePluginCustomization() {
    private val runtimeConfig = codegenContext.runtimeConfig
    private val smithyRuntime = RuntimeType.smithyRuntime(runtimeConfig)
    private val retries = smithyRuntime.resolve("client::retries")
    private val codegenScope = arrayOf(
        "TokenBucket" to retries.resolve("TokenBucket"),
        "TokenBucketPartition" to retries.resolve("TokenBucketPartition"),
        "ClientRateLimiter" to retries.resolve("ClientRateLimiter"),
        "ClientRateLimiterPartition" to retries.resolve("ClientRateLimiterPartition"),
        "StaticPartitionMap" to smithyRuntime.resolve("static_partition_map::StaticPartitionMap"),
    )

    override fun section(section: ServiceRuntimePluginSection): Writable = writable {
        when (section) {
            is ServiceRuntimePluginSection.DeclareSingletons -> {
                rustTemplate(
                    """
                    static TOKEN_BUCKET: #{StaticPartitionMap}<#{TokenBucketPartition}, #{TokenBucket}> = #{StaticPartitionMap}::new();
                    static CLIENT_RATE_LIMITER: #{StaticPartitionMap}<#{ClientRateLimiterPartition}, #{ClientRateLimiter}> = #{StaticPartitionMap}::new();
                    """,
                    *codegenScope,
                )
            }

            else -> emptySection
        }
    }
}
+0 −2
Original line number Diff line number Diff line
@@ -14,7 +14,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.customizations.Intercep
import software.amazon.smithy.rust.codegen.client.smithy.customizations.MetadataCustomization
import software.amazon.smithy.rust.codegen.client.smithy.customizations.ResiliencyConfigCustomization
import software.amazon.smithy.rust.codegen.client.smithy.customizations.ResiliencyReExportCustomization
import software.amazon.smithy.rust.codegen.client.smithy.customizations.ResiliencyServiceRuntimePluginCustomization
import software.amazon.smithy.rust.codegen.client.smithy.customizations.RetryClassifierConfigCustomization
import software.amazon.smithy.rust.codegen.client.smithy.customizations.RetryClassifierOperationCustomization
import software.amazon.smithy.rust.codegen.client.smithy.customizations.RetryClassifierServiceRuntimePluginCustomization
@@ -113,7 +112,6 @@ class RequiredCustomizations : ClientCodegenDecorator {
        codegenContext: ClientCodegenContext,
        baseCustomizations: List<ServiceRuntimePluginCustomization>,
    ): List<ServiceRuntimePluginCustomization> = baseCustomizations +
        ResiliencyServiceRuntimePluginCustomization(codegenContext) +
        ConnectionPoisoningRuntimePluginCustomization(codegenContext) +
        RetryClassifierServiceRuntimePluginCustomization(codegenContext)
}
+28 −11
Original line number Diff line number Diff line
@@ -40,7 +40,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.stripOuter
import software.amazon.smithy.rust.codegen.core.rustlang.withBlockTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
@@ -50,9 +49,11 @@ import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata
import software.amazon.smithy.rust.codegen.core.smithy.generators.getterName
import software.amazon.smithy.rust.codegen.core.smithy.generators.setterName
import software.amazon.smithy.rust.codegen.core.smithy.rustType
import software.amazon.smithy.rust.codegen.core.util.dq
import software.amazon.smithy.rust.codegen.core.util.inputShape
import software.amazon.smithy.rust.codegen.core.util.orNull
import software.amazon.smithy.rust.codegen.core.util.outputShape
import software.amazon.smithy.rust.codegen.core.util.sdkId
import software.amazon.smithy.rust.codegen.core.util.toSnakeCase

class FluentClientGenerator(
@@ -161,7 +162,7 @@ class FluentClientGenerator(
                }
                """,
                *clientScope,
                "base_client_runtime_plugins" to baseClientRuntimePluginsFn(runtimeConfig),
                "base_client_runtime_plugins" to baseClientRuntimePluginsFn(codegenContext),
            )
        }

@@ -446,8 +447,10 @@ class FluentClientGenerator(
    }
}

private fun baseClientRuntimePluginsFn(runtimeConfig: RuntimeConfig): RuntimeType =
private fun baseClientRuntimePluginsFn(codegenContext: ClientCodegenContext): RuntimeType = codegenContext.runtimeConfig.let { rc ->
    RuntimeType.forInlineFun("base_client_runtime_plugins", ClientRustModule.config) {
        val api = RuntimeType.smithyRuntimeApi(rc)
        val rt = RuntimeType.smithyRuntime(rc)
        rustTemplate(
            """
            pub(crate) fn base_client_runtime_plugins(
@@ -455,13 +458,25 @@ private fun baseClientRuntimePluginsFn(runtimeConfig: RuntimeConfig): RuntimeTyp
            ) -> #{RuntimePlugins} {
                let mut configured_plugins = #{Vec}::new();
                ::std::mem::swap(&mut config.runtime_plugins, &mut configured_plugins);

                let defaults = [
                    #{default_http_client_plugin}(),
                    #{default_retry_config_plugin}(${codegenContext.serviceShape.sdkId().dq()}),
                    #{default_sleep_impl_plugin}(),
                    #{default_time_source_plugin}(),
                    #{default_timeout_config_plugin}(),
                ].into_iter().flatten();

                let mut plugins = #{RuntimePlugins}::new()
                    .with_client_plugin(#{default_http_client_plugin}())
                    // defaults
                    .with_client_plugins(defaults)
                    // user config
                    .with_client_plugin(
                        #{StaticRuntimePlugin}::new()
                            .with_config(config.config.clone())
                            .with_runtime_components(config.runtime_components.clone())
                    )
                    // codegen config
                    .with_client_plugin(crate::config::ServiceRuntimePlugin::new(config))
                    .with_client_plugin(#{NoAuthRuntimePlugin}::new());
                for plugin in configured_plugins {
@@ -471,15 +486,17 @@ private fun baseClientRuntimePluginsFn(runtimeConfig: RuntimeConfig): RuntimeTyp
            }
            """,
            *preludeScope,
            "RuntimePlugins" to RuntimeType.runtimePlugins(runtimeConfig),
            "NoAuthRuntimePlugin" to RuntimeType.smithyRuntime(runtimeConfig)
                .resolve("client::auth::no_auth::NoAuthRuntimePlugin"),
            "StaticRuntimePlugin" to RuntimeType.smithyRuntimeApi(runtimeConfig)
                .resolve("client::runtime_plugin::StaticRuntimePlugin"),
            "default_http_client_plugin" to RuntimeType.smithyRuntime(runtimeConfig)
                .resolve("client::http::default_http_client_plugin"),
            "default_http_client_plugin" to rt.resolve("client::defaults::default_http_client_plugin"),
            "default_retry_config_plugin" to rt.resolve("client::defaults::default_retry_config_plugin"),
            "default_sleep_impl_plugin" to rt.resolve("client::defaults::default_sleep_impl_plugin"),
            "default_timeout_config_plugin" to rt.resolve("client::defaults::default_timeout_config_plugin"),
            "default_time_source_plugin" to rt.resolve("client::defaults::default_time_source_plugin"),
            "NoAuthRuntimePlugin" to rt.resolve("client::auth::no_auth::NoAuthRuntimePlugin"),
            "RuntimePlugins" to RuntimeType.runtimePlugins(rc),
            "StaticRuntimePlugin" to api.resolve("client::runtime_plugin::StaticRuntimePlugin"),
        )
    }
}

/**
 * For a given `operation` shape, return a list of strings where each string describes the name and input type of one of
+1 −1
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ internal class ResiliencyConfigCustomizationTest {
        project.withModule(ClientRustModule.config) {
            ServiceRuntimePluginGenerator(codegenContext).render(
                this,
                listOf(ResiliencyServiceRuntimePluginCustomization(codegenContext)),
                emptyList(),
            )
        }
        ResiliencyReExportCustomization(codegenContext).extras(project)
Loading