Make service config just contain a `FrozenLayer` in the orchestrator mode #2762

ysaito1001 · 2023-06-12T17:27:00Z

Motivation and Context

Service config structs now only contain a aws_smithy_types::config_bag::FrozenLayer in the orchestrator mode (no changes for the middleware mode).

Description

This PR reduces the individual fields of service configs to contain just FrozenLayer. This makes service configs work more seamlessly with runtime plugins in the orchestrator mode.

Note that service config builder s still contain individual fields. We're planning to make them just contain aws_smithy_types::config_bag::Layer. To do that, though, we need to make Layer cloneable and that will be handled in a separate PR. For builders, the only change you will in the PR is that their build method will put fields into a Layer, freeze it, and pass it to service configs.

This PR is marked as a breaking change because it's based on another PR that's also breaking change.

Testing

Passed tests in CI

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This commit updates service configs to contain only a single field `FrozenLayer` and remove individual fields. Note that their builders still contain individual fields instead of just containing a `Layer` because `Layer` is currently not `Clone`able.

github-actions · 2023-06-12T17:56:55Z

A new generated diff is ready to view.

A new doc preview is ready to view.

This commit modifies a Kotlin test for verifying dependencies brought in by an inlineable module. Prior to the commit, the test used an idempotency token, but with smithy-rs#2762, `IdempotencyTokenProvider` will bring in `aws_smithy_types` due to that provider being defined in an inlineable module. The test needed to be bifurcated between middleware and orchestrator but that was not possible because the test was in codegen-core and bifurcation was only possible in codegen-client. For this reason, this commit switches the test to use another inlineable module.

github-actions · 2023-06-12T20:06:51Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

...in/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt

...are/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGeneratorTest.kt

rust-runtime/aws-smithy-async/src/rt/sleep.rs

rcoh

Overall I really like the direction here.

Some things we should think about:

In a RuntimePlugin world, I don't think we need to set defaults in the service config do we? We might want a separate plugin that sets defaults and keep the fields on service config optional...
Some more thought about how we handle identity resolvers / interceptors is required I think
In a RuntimePlugin world, I guess service config only needs to handle fields we aren't taking care of elsewhere, maybe eventually, it just holds onto the SdkConfig in it's runtime plugin state?

...n/src/main/kotlin/software/amazon/smithy/rustsdk/customize/timestream/TimestreamDecorator.kt

...kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt

...ware/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt

...re/amazon/smithy/rust/codegen/client/smithy/customizations/InterceptorConfigCustomization.kt

...are/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt

…egen/client/smithy/generators/config/ServiceConfigGeneratorTest.kt Co-authored-by: Zelda Hessler <[email protected]>

github-actions · 2023-06-13T15:59:45Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

ysaito1001 · 2023-06-13T20:36:58Z

Overall I really like the direction here.

Some things we should think about:

1. In a RuntimePlugin world, I don't think we need to set defaults in the service config do we? We might want a separate plugin that sets defaults and keep the fields on service config optional...

2. Some more thought about how we handle identity resolvers / interceptors is required I think

3. In a RuntimePlugin world, I guess service config **only** needs to handle fields we aren't taking care of elsewhere, maybe eventually, it just holds onto the SdkConfig in it's runtime plugin state?

Shared with Russell off-line that this PR is meant to be as dumb and mechanical as possible, simply translating what we currently have into the new way. We'll address provided feedback in a series of upcoming PRs (especially 1 and 3 above).

This commit addresses #2762 (comment)

This commit addresses #2762 (comment) #2762 (comment)

github-actions · 2023-06-14T01:05:19Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

github-actions · 2023-06-14T02:40:27Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

github-actions · 2023-06-14T16:06:23Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

github-actions · 2023-06-15T17:47:35Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

jdisanti

Would it be simpler to use a ConfigBag in both middleware and orchestrator mode, and just change middleware code to call accessor methods instead?

...smithy/rust/codegen/client/smithy/generators/config/IdempotencyTokenProviderCustomization.kt

...hy/rust/codegen/client/smithy/generators/config/IdempotencyTokenProviderCustomizationTest.kt

This commit addresses #2762 (comment)

ysaito1001 · 2023-06-15T21:28:49Z

Would it be simpler to use a ConfigBag in both middleware and orchestrator mode, and just change middleware code to call accessor methods instead?

It's true that using ConfigBag in the middleware mode as well makes code changes simpler (since there's no need to bifurcate between two runtime modes), we should leave the middleware mode untouched in case there is a potential bug in ConfigBag we're not aware of.

github-actions · 2023-06-15T22:16:20Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

jdisanti · 2023-06-15T22:23:00Z

We should add a changelog entry for the make_token rename

Renaming `make_token` to `idempotency_token_provider` will be handled in a separate PR.

ysaito1001 · 2023-06-15T22:41:23Z

We should add a changelog entry for the make_token rename

Will do that as part of the next PR. Thanks for the suggestion.

github-actions · 2023-06-15T23:09:39Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

github-actions · 2023-06-15T23:49:12Z

A new generated diff is ready to view.

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

A new doc preview is ready to view.

This commit addresses #2762 (comment)

## Motivation and Context Incorporates suggestion made in #2762 (comment) ## Description This PR renames `make_token` to `idempotency_token_provider` for clarity wherever it is referred to in the fields and API in service configs and their builders. ## Testing Existing tests in CI ## Checklist  - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] 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._ --------- Co-authored-by: Yuki Saito <[email protected]>

This commit addresses #2762 (comment)

## Motivation and Context This PR will reduce fields in service config builders to contain only `CloneableLayer` (except for `interceptors` and `identity_resolvers`). ## Description This is the third PR in a series of config refactoring in the orchestrator mode. Just like #2762, this PR reduces fields in service config builders to contain `CloneableLayer`. The code changes follow a straightforward pattern where builders' setters are implemented via a config layer. ## Testing Relies on the existing tests in CI ---- _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: Zelda Hessler <[email protected]>

## Motivation and Context Incorporates suggestion made in smithy-lang/smithy-rs#2762 (comment) ## Description This PR renames `make_token` to `idempotency_token_provider` for clarity wherever it is referred to in the fields and API in service configs and their builders. ## Testing Existing tests in CI ## Checklist  - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] 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._ --------- Co-authored-by: Yuki Saito <[email protected]>

ysaito1001 added 3 commits June 9, 2023 22:54

Make types storable in config layer

189f2b5

Update tests to work with layer-backed service config

3b1b71a

ysaito1001 added 2 commits June 12, 2023 14:03

Merge branch 'main' into ysaito/frozen-layer-backed-service-config

a69a278

ysaito1001 marked this pull request as ready for review June 12, 2023 19:15

ysaito1001 requested review from a team as code owners June 12, 2023 19:15

ysaito1001 added the needs-sdk-review label Jun 12, 2023

Velfi reviewed Jun 13, 2023

View reviewed changes

rcoh reviewed Jun 13, 2023

View reviewed changes

Update codegen-client/src/test/kotlin/software/amazon/smithy/rust/cod…

f98f99d

…egen/client/smithy/generators/config/ServiceConfigGeneratorTest.kt Co-authored-by: Zelda Hessler <[email protected]>

ysaito1001 added 4 commits June 13, 2023 16:41

Merge branch 'main' into ysaito/frozen-layer-backed-service-config

3471a62

Remove unnecessary bifurcation for accessing time_source

7333e70

This commit addresses #2762 (comment)

Leave identity_resolvers and interceptors as fields

b988ad9

This commit addresses #2762 (comment)

Avoid unsetting fields during the build method

52a0d6e

This commit addresses #2762 (comment) #2762 (comment)

ysaito1001 mentioned this pull request Jun 14, 2023

Handle defaults in service config in a uniform manner in the orchestrator #2770

Closed

ysaito1001 added 2 commits June 13, 2023 20:59

Update externl-types.toml based on the latest main

f7b96ad

Fix redundant-clone clippy warning

303c90a

Merge branch 'main' into ysaito/frozen-layer-backed-service-config

2461a7f

Update service config and builder in ApiKeyAuthDecorator

7b9b56a

jdisanti reviewed Jun 15, 2023

View reviewed changes

ysaito1001 added the breaking-change This will require a breaking change label Jun 15, 2023

ysaito1001 added 2 commits June 15, 2023 15:51

Update make_token to token_provider

89185fa

This commit addresses #2762 (comment)

Add aws-smithy-types dep to idempotency_token inlineable

0e77721

This commit addresses #2762 (comment)

Update call site of make_token to use token_provider

c449eb4

Revert token_provider to make_token

8d573f2

Renaming `make_token` to `idempotency_token_provider` will be handled in a separate PR.

Merge branch 'main' into ysaito/frozen-layer-backed-service-config

79aa1ed

jdisanti approved these changes Jun 15, 2023

View reviewed changes

jdisanti enabled auto-merge June 15, 2023 23:43

jdisanti added this pull request to the merge queue Jun 15, 2023

Merged via the queue into main with commit 2e472d0 Jun 16, 2023

jdisanti deleted the ysaito/frozen-layer-backed-service-config branch June 16, 2023 00:09

ysaito1001 added a commit that referenced this pull request Jun 16, 2023

Rename make_token to idempotency_token_provider

7b187b8

This commit addresses #2762 (comment)

ysaito1001 mentioned this pull request Jun 16, 2023

Rename make_token to idempotency_token_provider #2783

Merged

2 tasks

ysaito1001 added a commit that referenced this pull request Jun 16, 2023

Rename make_token to idempotency_token_provider

6a9a580

This commit addresses #2762 (comment)

ysaito1001 mentioned this pull request Jun 21, 2023

Make service config builder contain CloneableLayer #2795

Merged

Velfi removed the needs-sdk-review label Oct 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make service config just contain a `FrozenLayer` in the orchestrator mode #2762

Make service config just contain a `FrozenLayer` in the orchestrator mode #2762

ysaito1001 commented Jun 12, 2023 •

edited

Loading

github-actions bot commented Jun 12, 2023

github-actions bot commented Jun 12, 2023

rcoh left a comment

github-actions bot commented Jun 13, 2023

ysaito1001 commented Jun 13, 2023

github-actions bot commented Jun 14, 2023

github-actions bot commented Jun 14, 2023

github-actions bot commented Jun 14, 2023

github-actions bot commented Jun 15, 2023

jdisanti left a comment

ysaito1001 commented Jun 15, 2023

github-actions bot commented Jun 15, 2023

jdisanti commented Jun 15, 2023

ysaito1001 commented Jun 15, 2023

github-actions bot commented Jun 15, 2023

github-actions bot commented Jun 15, 2023

Make service config just contain a FrozenLayer in the orchestrator mode #2762

Make service config just contain a FrozenLayer in the orchestrator mode #2762

Conversation

ysaito1001 commented Jun 12, 2023 • edited Loading

Motivation and Context

Description

Testing

github-actions bot commented Jun 12, 2023

github-actions bot commented Jun 12, 2023

rcoh left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 13, 2023

ysaito1001 commented Jun 13, 2023

github-actions bot commented Jun 14, 2023

github-actions bot commented Jun 14, 2023

github-actions bot commented Jun 14, 2023

github-actions bot commented Jun 15, 2023

jdisanti left a comment

Choose a reason for hiding this comment

ysaito1001 commented Jun 15, 2023

github-actions bot commented Jun 15, 2023

jdisanti commented Jun 15, 2023

ysaito1001 commented Jun 15, 2023

github-actions bot commented Jun 15, 2023

github-actions bot commented Jun 15, 2023

Make service config just contain a `FrozenLayer` in the orchestrator mode #2762

Make service config just contain a `FrozenLayer` in the orchestrator mode #2762

ysaito1001 commented Jun 12, 2023 •

edited

Loading