-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for constructing sdk body types from http-body 1.0 #3300
Conversation
A new generated diff is ready to view.
A new doc preview is ready to view. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
@@ -13,9 +13,10 @@ repository = "https://github.com/smithy-lang/smithy-rs" | |||
[features] | |||
byte-stream-poll-next = [] | |||
http-body-0-4-x = ["dep:http-body-0-4"] | |||
http-body-1-x = ["dep:http-body-1-0", "dep:http-body-util", "http-body-0-4-x"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this pulls in http-body-0-4-x
, then it will have to pull that in forever since removing it would technically break the API. It should pull in the dependency instead so that the APIs associated with the feature don't come with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point—these things were required to actually compile this in all cases but I'll add private APIs to fix this up.
"dep:http-body-0-4", | ||
"http-body-0-4-x", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will permanently tie rt-tokio to the http-body-0-4-x feature since it brings in the API for 0.4.
/// Construct an `SdkBody` from a type that implements [`http_body_1_0::Body<Data = Bytes>`](http_body_1_0::Body). | ||
/// | ||
/// _Note: This is only available with `http-body-1-0` enabled._ | ||
pub fn from_body_1_0<T, E>(body: T) -> Self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1_0
or 1_x
?
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. |
[features] | ||
byte-stream-poll-next = [] | ||
http-body-0-4-x = ["dep:http-body-0-4"] | ||
http-body-0-4-x = ["http-body-0-4"] | ||
http-body-1-x = ["dep:http-body-1-0", "dep:http-body-util", "http-body-0-4", "http-1x"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http-body-1-x = ["dep:http-body-1-0", "dep:http-body-util", "http-body-0-4", "http-1x"] | |
http-body-1-x = ["dep:http-body-1-0", "dep:http-body-util", "dep:http-body-0-4", "dep:http-1x"] |
#[cfg(any( | ||
feature = "http-body-0-4-x", | ||
feature = "http-body-1-x", | ||
feature = "rt-tokio" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does rt-tokio
require this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rt-tokio is what enables PathBody
, path body uses it
A new generated diff is ready to view.
A new doc preview is ready to view. |
impl SdkBody { | ||
/// Construct an `SdkBody` from a type that implements [`http_body_1_0::Body<Data = Bytes>`](http_body_1_0::Body). | ||
/// | ||
/// _Note: This is only available with `http-body-1-0` enabled._ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not suggesting to remove this (good to be explicit), but will #![cfg_attr(docsrs, feature(doc_auto_cfg))]
add a similar comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah should remove actually, I just copy-pasted but we'll get a docs attr
/// Construct a `ByteStream` from a type that implements [`http_body_0_4::Body<Data = Bytes>`](http_body_0_4::Body). | ||
/// | ||
/// _Note: This is only available with `http-body-0-4-x` enabled._ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Construct a `ByteStream` from a type that implements [`http_body_0_4::Body<Data = Bytes>`](http_body_0_4::Body). | |
/// | |
/// _Note: This is only available with `http-body-0-4-x` enabled._ | |
/// Construct a `ByteStream` from a type that implements [`http_body_1_0::Body<Data = Bytes>`](http_body_1_0::Body). | |
/// | |
/// _Note: This is only available with `http-body-1-0` enabled._ |
A new generated diff is ready to view.
A new doc preview is ready to view. |
## Motivation and Context - aws-sdk-rust#977 ## Description The first of several PRs to make add support for Hyper 1.0. This minimal change allows the use of Hyper 1.0 bodies although it does not actually leverage Hyper 1.0. ## Testing - Unit test suite - Ran cargo hack check --feature-powerset because we had a lot of features. I found a couple of issues. ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [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: John DiSanti <[email protected]>
Motivation and Context
Description
The first of several PRs to make add support for Hyper 1.0. This minimal change allows the use of Hyper 1.0 bodies although it does not actually leverage Hyper 1.0.
Testing
Checklist
CHANGELOG.next.toml
if I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.toml
if I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.