Add support for BehaviorMajorVersions #3151

rcoh · 2023-11-08T15:35:45Z

Motivation and Context

See rendered RFC

Description

This add BehaviorMajorVersions to the SDK and wires them in up and down the stack.

Testing

lots of ITs / UTs

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.

github-actions · 2023-11-08T15:54:06Z

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-11-08T17:42:17Z

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-11-08T18:05:49Z

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-11-09T15:35: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.

github-actions · 2023-11-09T16:43:46Z

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

Overall, I like the direction this is headed. Nice work!

jdisanti · 2023-11-09T19:15:33Z

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

+                    ///
+                    /// Over time, new best-practice behaviors are introduced. However, these behaviors might not be backwards
+                    /// compatible. For example, a change which introduces new default timeouts or a new retry-mode for
+                    /// all operations might be the ideal behavior but could break existing applications.


Add: If this is a concern, then explicitly set the version you want rather than using latest. Then you can upgrade to newer behaviors and do adequate testing at your leisure.

jdisanti · 2023-11-09T20:12:50Z

design/src/rfcs/rfc0039_behavior_major_versions.md

+<!-- The "Terminology" section is optional but is really useful for defining the technical terms you're using in the RFC -->
+Terminology
+-----------
+
+- **Some Term**: A definition for that term


Delete it if you're not going to fill it in

jdisanti · 2023-11-09T20:14:12Z

design/src/rfcs/rfc0039_behavior_major_versions.md

+
+If no `BehaviorMajorVersion` is set, the client will panic during construction.
+
+`BehaviorMajorVersion` is an opaque struct with initializers like `::latest()`, `::v2023_11_09()`. Downstream code can check the version by calling methods like `::supports_v1()`


I don't think the supports_*() would work or be useful since the function wouldn't exist in a sufficiently old SDK. I think if we were to support something like this, we would need to have from_str and supports_str methods on the BMV.

sufficently old SDKs won't have the newer code so there's no new versions to suppress

jdisanti · 2023-11-09T20:16:40Z

design/src/rfcs/rfc0039_behavior_major_versions.md

+}
+```
+
+We also will add `BehaviorVersionLatestRuntimePlugin`, a runtime plugin that is enabled when the `behavior-version-latest` feature is enabled in runtime crates.


Seems to me that these BMV plugins should be aggregate plugins that delegate to a bunch of other default RPs. Then the default RPs would no longer be configured on the client, but rather, just a single BMV plugin. That will cut down a lot on Arc<dyn> thrash since the BMV can directly refer to the RP type rather than making it into a trait object.

Separately, are there any use cases for registering a normal or override-level RP with BMVs?

jdisanti · 2023-11-09T20:19:17Z

rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs

+    /// If, however, you're writing a service that is very latency sensitive, or that has written
+    /// code to tune Rust SDK behaviors, consider pinning to a specific major version.
+    ///
+    /// The latest version is currently [`BehaviorMajorVersion::v1`]


Suggested change

/// The latest version is currently [`BehaviorMajorVersion::v1`]

/// The latest version is currently [`BehaviorMajorVersion::v2023_11_09`]

jdisanti · 2023-11-09T20:19:38Z

rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs

+        Self {}
+    }
+
+    /// This method returns the behavior configuration for November 9th 2023


Suggested change

/// This method returns the behavior configuration for November 9th 2023

/// This method returns the behavior configuration for November 9th, 2023

jdisanti · 2023-11-09T20:22:26Z

rust-runtime/aws-smithy-runtime-api/src/http/request.rs

@@ -337,7 +337,7 @@ mod test {
            .headers_mut()
            .try_insert("a\nb", "a\nb")
            .expect_err("invalid header");
-        let _ = req.headers_mut().insert("a\nb", "a\nb");
+        //let _ = req.headers_mut().insert("a\nb", "a\nb");


jdisanti · 2023-11-09T23:05:54Z

From offline conversation: We decided the BMV shouldn't be a runtime component nor in the config bag because there should be no way to change it from inside of orchestration. It needs to be a special field in the service config, separate from components and the layer. Then it can be passed in as a constructor arg to the runtime plugins that need it, and those runtime plugins can add things to the config bag to tell components further downstream how to behave.

ysaito1001

Love the idea of pinning when loading defaults.

ysaito1001 · 2023-11-13T19:08:29Z

design/src/rfcs/rfc0039_behavior_major_versions.md

+----------------------------------------------
+
+In the current version of the SDK, users can construct clients without indicating any sort of behavior major version.
+Once this RFC is implemented, there will be three ways to set a behavior major version:


Suggested change

Once this RFC is implemented, there will be three ways to set a behavior major version:

Once this RFC is implemented, there will be two ways to set a behavior major version:

ysaito1001 · 2023-11-13T19:10:53Z

rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs

+        Self {}
+    }
+
+    /// Returns whether V1 behaviors are enabled.


Suggested change

/// Returns whether V1 behaviors are enabled.

/// Returns whether V2023_11_09 behaviors are enabled.

github-actions · 2023-11-14T15:23:11Z

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-11-14T16:23:58Z

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-11-14T17:09:09Z

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-11-14T18:28:44Z

aws/rust-runtime/aws-config/src/default_provider/app_name.rs

@@ -117,7 +118,7 @@ mod tests {
    #[tokio::test]
    async fn profile_name_override() {
        let fs = Fs::from_slice(&[("test_config", "[profile custom]\nsdk_ua_app_id = correct")]);
-        let conf = crate::from_env()
+        let conf = crate::from_env_with_version(BehaviorMajorVersion::latest())


I think we should rename this to defaults_with_version, and load_from_env_with_version to load_defaults_with_version. In fact, maybe even remove the _with_version part.

jdisanti · 2023-11-14T18:48:38Z

design/src/rfcs/rfc0040_behavior_major_versions.md

+
+To help customers migrate, we are including `from_env` hooks that set `behavior-version-latest` that are _deprecated_. This allows customers to see that they are missing the required cargo feature and add it to remove the deprecation warning.
+
+Internally, `BehaviorMajorVersion` will become an additional field on `<client>::Config`. It is _not_ ever stored in the `ConfigBag` or in `RuntimePlugins`.


Would be nice to add how the BMV will be incorporated into RuntimePlugins via constructors and separate config bag values.

…-major-versions

github-actions · 2023-11-14T21:46:40Z

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.

rcoh marked this pull request as ready for review November 9, 2023 15:19

rcoh requested review from a team as code owners November 9, 2023 15:19

rcoh force-pushed the behavior-major-versions branch from 84568fe to a6e8c21 Compare November 9, 2023 18:18

jdisanti reviewed Nov 9, 2023

View reviewed changes

rcoh mentioned this pull request Nov 13, 2023

0.58 Tracking Issue #3185

Closed

3 tasks

rcoh force-pushed the behavior-major-versions branch from 4f398fa to 685afc1 Compare November 13, 2023 18:48

rcoh requested review from a team as code owners November 13, 2023 18:48

ysaito1001 approved these changes Nov 13, 2023

View reviewed changes

rcoh force-pushed the behavior-major-versions branch from 85e881a to d6a5881 Compare November 14, 2023 15:05

Add support for BehaviorMajorVersions

f40840d

rcoh force-pushed the behavior-major-versions branch from 877edf5 to 6220b61 Compare November 14, 2023 16:05

Remove StubConfigCustomization test facility and update tests

708c75f

rcoh force-pushed the behavior-major-versions branch from 6220b61 to 708c75f Compare November 14, 2023 16:47

CR feedback

cdccf26

jdisanti reviewed Nov 14, 2023

View reviewed changes

rcoh added 4 commits November 14, 2023 15:23

Use BehaviorVersionLatest in docs

1ac61ae

Merge branch 'main' of github.com:smithy-lang/smithy-rs into behavior…

73aad5e

…-major-versions

Clarify RFC

fa6f8ce

Remove terminology section

b7fd455

jdisanti approved these changes Nov 14, 2023

View reviewed changes

rcoh added 2 commits November 14, 2023 16:25

Fix compilation

9b2007d

Merge branch 'main' into behavior-major-versions

870fa2d

rcoh enabled auto-merge November 14, 2023 21:29

rcoh added this pull request to the merge queue Nov 14, 2023

Merged via the queue into main with commit 446326c Nov 14, 2023
40 of 41 checks passed

rcoh deleted the behavior-major-versions branch November 14, 2023 22:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for BehaviorMajorVersions #3151

Add support for BehaviorMajorVersions #3151

rcoh commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

github-actions bot commented Nov 9, 2023

github-actions bot commented Nov 9, 2023

jdisanti left a comment

jdisanti Nov 9, 2023

jdisanti Nov 9, 2023

jdisanti Nov 9, 2023

rcoh Nov 14, 2023

jdisanti Nov 9, 2023

jdisanti Nov 9, 2023

jdisanti Nov 9, 2023

jdisanti Nov 9, 2023

jdisanti commented Nov 9, 2023

ysaito1001 left a comment

ysaito1001 Nov 13, 2023

ysaito1001 Nov 13, 2023

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

jdisanti Nov 14, 2023

jdisanti Nov 14, 2023

rcoh Nov 14, 2023

github-actions bot commented Nov 14, 2023


		If no `BehaviorMajorVersion` is set, the client will panic during construction.

		`BehaviorMajorVersion` is an opaque struct with initializers like `::latest()`, `::v2023_11_09()`. Downstream code can check the version by calling methods like `::supports_v1()`

	/// The latest version is currently [`BehaviorMajorVersion::v1`]
	/// The latest version is currently [`BehaviorMajorVersion::v2023_11_09`]

	/// This method returns the behavior configuration for November 9th 2023
	/// This method returns the behavior configuration for November 9th, 2023

	Once this RFC is implemented, there will be three ways to set a behavior major version:
	Once this RFC is implemented, there will be two ways to set a behavior major version:

	/// Returns whether V1 behaviors are enabled.
	/// Returns whether V2023_11_09 behaviors are enabled.


		To help customers migrate, we are including `from_env` hooks that set `behavior-version-latest` that are _deprecated_. This allows customers to see that they are missing the required cargo feature and add it to remove the deprecation warning.

		Internally, `BehaviorMajorVersion` will become an additional field on `<client>::Config`. It is _not_ ever stored in the `ConfigBag` or in `RuntimePlugins`.

Add support for BehaviorMajorVersions #3151

Add support for BehaviorMajorVersions #3151

Conversation

rcoh commented Nov 8, 2023 • edited Loading

Motivation and Context

Description

Testing

Checklist

github-actions bot commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

github-actions bot commented Nov 9, 2023

github-actions bot commented Nov 9, 2023

jdisanti left a 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

Choose a reason for hiding this comment

jdisanti commented Nov 9, 2023

ysaito1001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

github-actions bot commented Nov 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 14, 2023

rcoh commented Nov 8, 2023 •

edited

Loading