feat!: add interceptor client config #704

milesziemer · 2024-04-30T20:14:35Z

Issue #

Description of changes

Allows configuration of interceptors on a client level by adding interceptor
providers to client config, allowing Plugins to add interceptors.

The primary addition is InterceptorProvider, an interface that creates
generic interceptors which can operate on any transport - http or otherwise.
When an operation is executed, interceptor providers are called to create
new instances of service-level interceptors. Creating new instances also
means we don't need to synchronize on the shared interceptors. Transport
specific config, have their own methods for adding interceptor providers, so
you can add HttpInterceptorProviders to http config. Operations know which
transport they operate on, and can choose which transport-specific interceptor
providers to use.

If/when we have operation-level configuration, it might make more sense to
allow plugins to configure more generic 'operation customizations' or
something, rather than just the interceptors. Operations would then call
the customizations before executing.

A few other minor changes were made to the client libraries:

Added actual builder methods to RequestMessageBuilder, which we would need
eventually, so I could use them in testing
Made SdkHttpRequestBuilder final

Codegen was also updated to generate the new config methods, and to call
interceptor providers in operations to add configured interceptors.

Scope

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Technically a breaking change because it adds methods to config protocols, but it is a new feature

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

See smithy-lang/smithy-swift#704

Codegen for smithy-lang/smithy-swift#704. Also fixes a codegen test case.

AndrewFossAWS · 2024-05-16T15:47:43Z

Sources/ClientRuntime/Message/RequestMessageBuilder.swift

@@ -13,6 +13,10 @@ public protocol RequestMessageBuilder<RequestType>: AnyObject {

    init()

+    func withHost(_ host: String) -> Self


Host should be provided from URI?

Oh yea, this is just here so I could use it in a test. I think this can changed to something like withURI

dayaffe

Just one nit + please address Andrew's comment before merging

dayaffe · 2024-05-22T20:33:22Z

...lin/software/amazon/smithy/swift/codegen/integration/HttpProtocolUnitTestRequestGenerator.kt

                writer.write("let builder = OrchestratorBuilder<$inputSymbol, $outputSymbol, SdkHttpRequest, HttpResponse, HttpContext>()")
+                writer.write("config.interceptorProviders.forEach { builder.interceptors.add($$0.create()) }")


nit: more readable if you did

config.interceptorProviders.forEach { provider in builder.interceptors.add(provider.create()) }

dayaffe · 2024-05-22T20:38:00Z

.../main/kotlin/software/amazon/smithy/swift/codegen/middleware/MiddlewareExecutionGenerator.kt

+                config.httpInterceptorProviders.forEach {
+                    let i: any HttpInterceptor<${'$'}N, ${'$'}N> = $$0.create()
+                    builder.interceptors.add(i)
+                }


same nit here might be better to use a name instead of $0.create for readability

Codegen for smithy-lang/smithy-swift#704. Also fixes a codegen test case, and updates the types of some interceptor implementations to remove AttributesType.

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType.

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

dayaffe · 2024-06-24T16:09:57Z

Sources/ClientRuntime/Config/DefaultClientConfiguration.swift

@@ -31,5 +31,10 @@ public protocol DefaultClientConfiguration: ClientConfiguration {
    /// If none is provided, only a default logger provider will be used.
    var telemetryProvider: TelemetryProvider { get set }

+    /// Add an `InterceptorProvider` that will be used to provide interceptors for all operations.


Q: Interceptor providers apply to Http operations + all other operations vs Http Interceptor providers apply to only http operations?

That is correct

dayaffe · 2024-06-24T16:10:49Z

Sources/ClientRuntime/Interceptor/DefaultInterceptorContext.swift

-    public func updateResult(updated: Result<OutputType, Error>) {
-        self.result = updated
+    public func updateOutput(updated: OutputType) {
+        self.result = .success(updated)


before updated could be an Error, will this always be a success?

The orchestrator uses setResult to set errors. So internally DefaultInterceptorContext keeps track of failures - if you look at getOutput, you'll see it now throws.

This change was based on discussion with @jbelkins about mixing and matching Result with throws in Swift.

dayaffe · 2024-06-24T16:11:54Z

Sources/ClientRuntime/Interceptor/DefaultInterceptorContext.swift

        return self.attributes
    }
+


please take care of the lint issues

Maybe we should make these lint issues errors instead of warnings so they fail local swiftlint runs and CI

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

Allows configuration of interceptors on a client level by adding interceptor providers to client config, allowing Plugins to add interceptors. The primary addition is `InterceptorProvider`, an interface that creates generic interceptors which can operate on any transport - http or otherwise. When an operation is executed, interceptor providers are called to create new instances of service-level interceptors. Creating new instances also means we don't need to synchronize on the shared interceptors. Transport specific config, have their own methods for adding interceptor providers, so you can add `HttpInterceptorProvider`s to http config. Operations know which transport they operate on, and can choose which transport-specific interceptor providers to use. If/when we have operation-level configuration, it might make more sense to allow plugins to configure more generic 'operation customizations' or something, rather than just the interceptors. Operations would then call the customizations before executing. A few other minor changes were made to the client libraries: - Removed HasAttributes protocol and corresponding AttributesType from interceptor interfaces, as we now just use `Context`. - Changed InterceptorContext `getResult` to `getOutput`. We still store a `Result<OutputType, Error>` in DefaultInterceptorContext, but `getOutput` now throws that error if it is present, otherwise just returns `OutputType` - Added actual builder methods to RequestMessageBuilder, which we would need eventually, so I could use them in testing - Made SdkHttpRequestBuilder final Codegen was also updated to generate the new config methods, and to call interceptor providers in operations to add configured interceptors.

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

milesziemer force-pushed the interceptors-config branch from f269732 to 0bbf847 Compare May 9, 2024 18:04

milesziemer marked this pull request as ready for review May 9, 2024 18:08

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request May 9, 2024

chore: fix test case for interceptors config

69c85ea

See smithy-lang/smithy-swift#704

milesziemer mentioned this pull request May 9, 2024

chore: codegen interceptors config awslabs/aws-sdk-swift#1493

Merged

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request May 9, 2024

chore: codegen interceptors config

e8616dc

Codegen for smithy-lang/smithy-swift#704. Also fixes a codegen test case.

milesziemer force-pushed the interceptors-config branch 2 times, most recently from fbcac8e to 8e4ec7c Compare May 10, 2024 16:51

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request May 13, 2024

chore: codegen interceptors config

9fe52c6

Codegen for smithy-lang/smithy-swift#704. Also fixes a codegen test case.

milesziemer force-pushed the interceptors-config branch from 8e4ec7c to 3448636 Compare May 14, 2024 15:28

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request May 14, 2024

chore: codegen interceptors config

40c274b

Codegen for smithy-lang/smithy-swift#704. Also fixes a codegen test case.

AndrewFossAWS reviewed May 16, 2024

View reviewed changes

milesziemer requested review from dayaffe and aws-dash and removed request for aws-dash May 17, 2024 16:28

dayaffe approved these changes May 22, 2024

View reviewed changes

dayaffe reviewed May 22, 2024

View reviewed changes

milesziemer force-pushed the interceptors-config branch from 3448636 to 0831020 Compare June 11, 2024 18:54

milesziemer force-pushed the interceptors-config branch from 0831020 to 1db4723 Compare June 11, 2024 19:03

milesziemer force-pushed the interceptors-config branch 3 times, most recently from 350997f to bdbfa0b Compare June 12, 2024 19:35

milesziemer force-pushed the interceptors-config branch from bdbfa0b to 263ef6e Compare June 12, 2024 21:29

milesziemer force-pushed the interceptors-config branch from 263ef6e to a0d018f Compare June 20, 2024 20:08

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request Jun 20, 2024

chore: codegen interceptors config

2b53eec

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType.

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request Jun 20, 2024

chore: codegen interceptors config

8be8b54

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType.

milesziemer force-pushed the interceptors-config branch from a0d018f to 1c9d7ba Compare June 21, 2024 15:49

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request Jun 21, 2024

chore: codegen interceptors config

24d0ce1

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request Jun 21, 2024

chore: codegen interceptors config

357d10b

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

milesziemer force-pushed the interceptors-config branch from 1c9d7ba to cb09e6f Compare June 24, 2024 14:03

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request Jun 24, 2024

chore: codegen interceptors config

6cac7e9

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

milesziemer force-pushed the interceptors-config branch from cb09e6f to 157b12f Compare June 24, 2024 14:45

dayaffe self-requested a review June 24, 2024 14:46

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request Jun 24, 2024

chore: codegen interceptors config

95be154

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

dayaffe reviewed Jun 24, 2024

View reviewed changes

milesziemer force-pushed the interceptors-config branch from 157b12f to 84261fe Compare June 24, 2024 16:20

dayaffe self-requested a review June 24, 2024 18:44

dayaffe approved these changes Jun 24, 2024

View reviewed changes

milesziemer force-pushed the interceptors-config branch from 84261fe to a0efc36 Compare June 24, 2024 18:50

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request Jun 24, 2024

chore: codegen interceptors config

8f304d2

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

milesziemer force-pushed the interceptors-config branch from a0efc36 to 4645747 Compare June 26, 2024 15:30

milesziemer added a commit to awslabs/aws-sdk-swift that referenced this pull request Jun 26, 2024

chore: codegen interceptors config

67311f1

Codegen for smithy-lang/smithy-swift#704. Also updates the types of some interceptor implementations to remove AttributesType, and fixes a test case.

milesziemer merged commit 273e1be into main Jun 26, 2024
27 checks passed

milesziemer deleted the interceptors-config branch June 26, 2024 16:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: add interceptor client config #704

feat!: add interceptor client config #704

milesziemer commented Apr 30, 2024 •

edited

Loading

AndrewFossAWS May 16, 2024

milesziemer May 17, 2024

dayaffe left a comment •

edited

Loading

dayaffe May 22, 2024

dayaffe May 22, 2024

dayaffe Jun 24, 2024

milesziemer Jun 24, 2024

dayaffe Jun 24, 2024

milesziemer Jun 24, 2024

dayaffe Jun 24, 2024

milesziemer Jun 24, 2024

		@@ -13,6 +13,10 @@ public protocol RequestMessageBuilder<RequestType>: AnyObject {

		init()

		func withHost(_ host: String) -> Self

		writer.write("let builder = OrchestratorBuilder<$inputSymbol, $outputSymbol, SdkHttpRequest, HttpResponse, HttpContext>()")
		writer.write("config.interceptorProviders.forEach { builder.interceptors.add($$0.create()) }")

feat!: add interceptor client config #704

feat!: add interceptor client config #704

Conversation

milesziemer commented Apr 30, 2024 • edited Loading

Issue #

Description of changes

Scope

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dayaffe left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milesziemer commented Apr 30, 2024 •

edited

Loading

dayaffe left a comment •

edited

Loading