update AWS runtime crates for request compression #3632
Conversation
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
|
||
| #[tokio::test] | ||
| #[traced_test] | ||
| async fn environment_priority() { |
There was a problem hiding this comment.
I believe EnvConfigValue itself has already been tested that way, but maybe we should also add a test making sure a value is correctly loaded from a profile config when disabling is only specified in a shared config file, but not set in the environment variable?
|
|
||
| #[tokio::test] | ||
| #[traced_test] | ||
| async fn environment_priority() { |
There was a problem hiding this comment.
The above comment regarding testing against a shared config file also applies here?
| @@ -473,7 +473,7 @@ mod tests { | |||
| use crate::sensitive_command::CommandWithSensitiveArgs; | |||
| use serde::Deserialize; | |||
|
|
|||
| #[cfg(feature = "test-utils")] | |||
| #[cfg(feature = "test-util")] | |||
There was a problem hiding this comment.
Is this just a name change without any programmatic effects?
There was a problem hiding this comment.
Beyond the name change, the aws-config crate doesn't seem to have a test-util(s) feature listed in Cargo.toml at all so I'm not sure how this is working. Does it transitively work with the test-util feature present in some of the other crates?
There was a problem hiding this comment.
We never added these features to the cargo.toml so I'm pretty sure they were never usable. I changed the name to bring it in line with the other crates. I'm also going to add the feature to the Cargo.toml.
There was a problem hiding this comment.
I'm going to transfer this change to a separate PR because it's causing some test failures.
| @@ -41,6 +41,25 @@ pub(crate) fn parse_bool(value: &str) -> Result<bool, InvalidBooleanValue> { | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Debug)] | |||
| pub(crate) struct InvalidUintValue { | |||
There was a problem hiding this comment.
I assume we only enforce non_exhaustive if a struct is public outside of its containing crate?
There was a problem hiding this comment.
Correct. If it's not public then we don't have to worry about forwards compatibility.
| @@ -473,7 +473,7 @@ mod tests { | |||
| use crate::sensitive_command::CommandWithSensitiveArgs; | |||
| use serde::Deserialize; | |||
|
|
|||
| #[cfg(feature = "test-utils")] | |||
| #[cfg(feature = "test-util")] | |||
There was a problem hiding this comment.
Beyond the name change, the aws-config crate doesn't seem to have a test-util(s) feature listed in Cargo.toml at all so I'm not sure how this is working. Does it transitively work with the test-util feature present in some of the other crates?
| /// 2. The profile key `disable_request_compression=true/false` | ||
| /// | ||
| /// If invalid values are found, the provider will return None and an error will be logged. | ||
| pub async fn disable_request_compression_provider( |
There was a problem hiding this comment.
Seems odd to me these end up being public to the world but that seems to be the pattern here. I would have thought pub crate.
There was a problem hiding this comment.
I was doing it to match the other providers but I don't mind making them pub(crate). We can always undo that later if we decide to.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This PR includes all the changes necessary for AWS runtime crates to support request compression.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.