Feature: Customizable Operations #1647

Velfi · 2022-08-18T20:17:49Z

Motivation and Context

Description

SDK customers occasionally need to add additional HTTP headers to requests, and currently,
the SDK has previously had no easy way to accomplish this. This PR adds a feature that provides an easier way to augment requests that is compatible with the fluent client.

Before:

let input = SomeOperationInput::builder().some_value(5).build()?;

let operation = {
    let op = input.make_operation(&service_config).await?;
    let (request, response) = op.into_request_response();

    let request = request.augment(|req, _props| {
        req.headers_mut().insert(
            HeaderName::from_static("x-some-header"),
            HeaderValue::from_static("some-value")
        );
        Result::<_, Infallible>::Ok(req)
    })?;

    Operation::from_parts(request, response)
};

let response = smithy_client.call(operation).await?;

After:

let response = client.some_operation()
    .some_value(5)
    .customize()
    .await?
    .mutate_request(|mut req| {
        req.headers_mut().insert(
            HeaderName::from_static("x-some-header"),
            HeaderValue::from_static("some-value")
        );
    })
    .send()
    .await?;

Major credit to @jdisanti who wrote the original RFC for this feature.

Testing

I wrote an integration test

Checklist

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.

Velfi · 2022-08-18T20:20:42Z

We didn't previously have an inlineables module for smithy stuff so I created one as part of this PR. In the process, I copied the current AWS inlineables implementation closely and I may have done things in a strange way. I look forward to y'alls comments and criticism.

github-actions · 2022-08-18T20:49:07Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

…operations

Velfi · 2022-08-19T16:54:06Z

The test failures are due to the feature being defined in an inlineable. I'll have to move it into codegen unless we change how we handle the smithy client generics. I'm going to ask around internally to get a feel for what people think about that.

github-actions · 2022-08-19T17:00:27Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

jdisanti

Exciting!

CHANGELOG.next.toml

...kotlin/software/amazon/smithy/rust/codegen/smithy/generators/client/FluentClientDecorator.kt

rust-runtime/aws-smithy-inlineable/Cargo.toml

rust-runtime/build.gradle.kts

…operations

remove: unnecessary build.gradle.kts change add: missing copyright headers

Velfi

Added some comments to delineate the new fluent client codegen from the old fluent client codegen

...kotlin/software/amazon/smithy/rust/codegen/smithy/generators/client/FluentClientGenerator.kt

github-actions · 2022-08-30T21:18:13Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

jdisanti

This looks good to me! Just a question and some simple fixes.

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryPolicyDecorator.kt

...n/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/GenericsGenerator.kt

...kotlin/software/amazon/smithy/rust/codegen/smithy/generators/client/FluentClientGenerator.kt

…operations

github-actions · 2022-09-01T14:52:20Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

update: add IntelliJ idea folder to .gitignore update: use nullable type instead of optional in GenericsGenerator update: use clearer naming and types in GenericsGenerator add: fancy formatter for generic type parameters update: use new type param formatter for customizable op generics

…operations

github-actions · 2022-09-01T18:35:55Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

…uentClientGenerator directly

github-actions · 2022-09-01T20:55:08Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

david-perez · 2022-09-01T20:25:52Z

.gitignore

@@ -45,3 +45,6 @@ gradle-app.setting

 # Rust build artifacts
 target/
+
+# IDEs
+.idea/


Missing newline.

david-perez · 2022-09-01T20:29:29Z

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsFluentClientDecorator.kt

+    override fun toGenericsGenerator(): GenericsGenerator {
+        return GenericsGenerator()
+    }


Suggested change

override fun toGenericsGenerator(): GenericsGenerator {

return GenericsGenerator()

}

override fun toGenericsGenerator() = GenericsGenerator()

david-perez · 2022-09-01T20:35:42Z

...n/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/GenericsGenerator.kt

+    private val typeArgs: MutableList<GenericTypeArg>
+
+    init {
+        typeArgs = genericTypeArgs.toMutableList()
+    }


Why not simply:

Suggested change

private val typeArgs: MutableList<GenericTypeArg>

init {

typeArgs = genericTypeArgs.toMutableList()

}

private val typeArgs = genericTypeArgs.toMutableList()

However, do we need mutability here? I don't see the add method being used.

...n/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/GenericsGenerator.kt

david-perez · 2022-09-01T20:42:21Z

...n/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/GenericsGenerator.kt

+    val bound: RuntimeType? = null,
+)
+
+class GenericsGenerator(vararg genericTypeArgs: GenericTypeArg) {


Thanks for creating this generator. We will reuse it for the server, we have a slew of generics and bounds in the operation registry.

nit: would be nice to have some docs on usage and unit tests.

...n/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/GenericsGenerator.kt

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/Writable.kt

.../kotlin/software/amazon/smithy/rust/codegen/smithy/generators/client/FluentClientGenerics.kt

update: use Unit instead of "()" update: RustSymbolFormatter to handle RustTypes add: tests for rustTypeParameters

github-actions · 2022-09-01T21:48:17Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

add: tests for GenericsGenerator update: tests for rustTypeParameters add: docs for GenericsGenerator refactor: rustInline to be rustInlineTemplate. Now, it can template, and won't trim input update: code per PR comments

github-actions · 2022-09-02T16:24:18Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

github-actions · 2022-09-02T22:16:58Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python

A new doc preview is ready to view.

Velfi · 2022-09-26T16:26:14Z

released as part of v0.49.0

## Motivation and Context Port [Customizable Operation](#1647) to orchestrator ## Description This PR implements `CustomizableOperation` in the orchestrator. Just like the counterpart in the middleware, it is created when the `customize` method (in the orchestrator mode) on a fluent builder is called. The `customize` method in the orchestrator could technically be made a synchronous method because there is no need to create an operation, which requires `async`, therefore making the `customize` method in the middleware `async`. However, during the transition from the middleware to the orchestrator, the integration tests ([example](https://github.com/awslabs/smithy-rs/blob/31c152d9af53afb9a5e6edf9df3def57931b9c1e/aws/sdk/integration-tests/s3/tests/signing-it.rs#L36)) need to be able to run in both modes. For this reason, the `customize` method in the orchestrator is temporarily marked as `async`. Regarding methods defined on the new `CustomizableOperation`, they include `mutate_request` and `map_request` from the counterpart in the middleware. However, it did not port `map_operation` because there is no operation to map on. Most use cases for `map_operation` is put things in a property bag. The new `CustomizableOperation` provides an `interceptor` method to accomplish the same, i.e putting things in a config bag. Finally, for integration tests to run in both modes, the code gen emits the implementation of the `customize` method differently depending on the active Smithy runtime mode, similar to what the implementation of `send` method does. ## Testing Added one `sra-test` for mutating a request. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: John DiSanti <[email protected]>

## Motivation and Context Port [Customizable Operation](smithy-lang/smithy-rs#1647) to orchestrator ## Description This PR implements `CustomizableOperation` in the orchestrator. Just like the counterpart in the middleware, it is created when the `customize` method (in the orchestrator mode) on a fluent builder is called. The `customize` method in the orchestrator could technically be made a synchronous method because there is no need to create an operation, which requires `async`, therefore making the `customize` method in the middleware `async`. However, during the transition from the middleware to the orchestrator, the integration tests ([example](https://github.com/awslabs/smithy-rs/blob/31c152d9af53afb9a5e6edf9df3def57931b9c1e/aws/sdk/integration-tests/s3/tests/signing-it.rs#L36)) need to be able to run in both modes. For this reason, the `customize` method in the orchestrator is temporarily marked as `async`. Regarding methods defined on the new `CustomizableOperation`, they include `mutate_request` and `map_request` from the counterpart in the middleware. However, it did not port `map_operation` because there is no operation to map on. Most use cases for `map_operation` is put things in a property bag. The new `CustomizableOperation` provides an `interceptor` method to accomplish the same, i.e putting things in a config bag. Finally, for integration tests to run in both modes, the code gen emits the implementation of the `customize` method differently depending on the active Smithy runtime mode, similar to what the implementation of `send` method does. ## Testing Added one `sra-test` for mutating a request. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: John DiSanti <[email protected]>

Velfi added 3 commits August 18, 2022 15:13

feature: customizable operations

3e9cf67

update: CHANGELOG.next.toml

9cedc99

update: RFC0017

da24b9f

Velfi requested a review from a team as a code owner August 18, 2022 20:17

Velfi mentioned this pull request Aug 18, 2022

[request] Allow overriding of request configuration on fluent request builders awslabs/aws-sdk-rust#537

Closed

2 tasks

Velfi added 2 commits August 19, 2022 09:43

fix: clippy lints

9ef1c6a

Merge remote-tracking branch 'origin/main' into feature/customizable-…

0c602c0

…operations

jdisanti reviewed Aug 19, 2022

View reviewed changes

Velfi added 2 commits August 30, 2022 16:00

update: customizable ops codegen to work for both AWS and native clients

eb9dd16

Merge remote-tracking branch 'origin/main' into feature/customizable-…

a96dd7e

…operations

Velfi requested review from crisidev and david-perez as code owners August 30, 2022 21:00

Velfi added 2 commits August 30, 2022 16:05

update: CHANGELOG.next.toml refs

0ff7ce1

remove: unnecessary build.gradle.kts change add: missing copyright headers

format: run kotlin formatter

bf21257

Velfi commented Aug 30, 2022

View reviewed changes

jdisanti approved these changes Aug 30, 2022

View reviewed changes

Merge remote-tracking branch 'origin/main' into feature/customizable-…

7a96dd5

…operations

Velfi added 2 commits September 1, 2022 13:15

Merge remote-tracking branch 'origin/main' into feature/customizable-…

039add4

…operations

Velfi added 2 commits September 1, 2022 14:21

refactor: move Writables into their own file

43375f2

refactor: remove RetryPolicy trait in favor of passing it into the Fl…

2dc6229

…uentClientGenerator directly

david-perez approved these changes Sep 1, 2022

View reviewed changes

add: Unit to RustType

8628478

update: use Unit instead of "()" update: RustSymbolFormatter to handle RustTypes add: tests for rustTypeParameters

remove: unused methods for GenericsGenerator

eca1296

add: tests for GenericsGenerator update: tests for rustTypeParameters add: docs for GenericsGenerator refactor: rustInline to be rustInlineTemplate. Now, it can template, and won't trim input update: code per PR comments

Velfi changed the title ~~Feature/customizable operations~~ Feature: Customizable Operations Sep 2, 2022

Merge branch 'main' into feature/customizable-operations

5ddbe7f

Velfi merged commit 50d88a5 into main Sep 2, 2022

Velfi deleted the feature/customizable-operations branch September 2, 2022 22:47

Velfi added the pending-release label Sep 8, 2022

jdisanti mentioned this pull request Sep 13, 2022

Users should be able to modify requests at a high level #1112

Closed

Velfi removed the pending-release label Sep 26, 2022

Velfi mentioned this pull request Feb 28, 2023

[Feature Request] Allow configuration of RetryClassifier on the client #2417

Closed

ysaito1001 mentioned this pull request May 17, 2023

Port customizable operation to orchestrator #2706

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Customizable Operations #1647

Feature: Customizable Operations #1647

Velfi commented Aug 18, 2022 •

edited

Loading

Velfi commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

Velfi commented Aug 19, 2022

github-actions bot commented Aug 19, 2022

jdisanti left a comment

Velfi left a comment

github-actions bot commented Aug 30, 2022

jdisanti left a comment

github-actions bot commented Sep 1, 2022

github-actions bot commented Sep 1, 2022

github-actions bot commented Sep 1, 2022

david-perez Sep 1, 2022

david-perez Sep 1, 2022

david-perez Sep 1, 2022

david-perez Sep 1, 2022

github-actions bot commented Sep 1, 2022

github-actions bot commented Sep 2, 2022

github-actions bot commented Sep 2, 2022

Velfi commented Sep 26, 2022

Feature: Customizable Operations #1647

Feature: Customizable Operations #1647

Conversation

Velfi commented Aug 18, 2022 • edited Loading

Motivation and Context

Description

Testing

Checklist

Velfi commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

Velfi commented Aug 19, 2022

github-actions bot commented Aug 19, 2022

jdisanti left a comment

Choose a reason for hiding this comment

Velfi left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 30, 2022

jdisanti left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 1, 2022

github-actions bot commented Sep 1, 2022

github-actions bot commented Sep 1, 2022

david-perez Sep 1, 2022

Choose a reason for hiding this comment

david-perez Sep 1, 2022

Choose a reason for hiding this comment

david-perez Sep 1, 2022

Choose a reason for hiding this comment

david-perez Sep 1, 2022

Choose a reason for hiding this comment

github-actions bot commented Sep 1, 2022

github-actions bot commented Sep 2, 2022

github-actions bot commented Sep 2, 2022

Velfi commented Sep 26, 2022

Velfi commented Aug 18, 2022 •

edited

Loading