Skip to content

Various fixes and updates for aws-smithy-http including errors and impl futures::Stream for SdkBody #1733

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

Closed
wants to merge 1 commit into from

Conversation

Velfi
Copy link
Contributor

@Velfi Velfi commented Sep 14, 2022

Motivation and Context

I started this because of #1582 but I added more changes.

Description

This fixes several small issues I had with aws-smithy-http including:
add: futures::Stream impl for SdkBody
update: use futures-lite instead of futures-core
remove: callbacks API for SdkBody and ByteStream
remove: Error::source impl for byte_stream::Error
update: hyper version

Testing

wrote 2 tests and ran existing tests

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.

add: futures::Stream impl for SdkBody
update: use futures-lite instead of futures-core
remove: callbacks API for SdkBody and ByteStream
remove: Error::source impl for byte_stream::Error
update: hyper version
@Velfi Velfi changed the title refactor: aws-smithy-http errors Various fixes and updates for aws-smithy-http including errors and impl futures::Stream for SdkBody Sep 14, 2022
@@ -26,7 +26,7 @@ aws-types = { path = "../../sdk/build/aws-sdk/sdk/aws-types" }
time = { version = "0.3.4", features = ["parsing"] }
tokio = { version = "1.8.4", features = ["sync"] }
tracing = { version = "0.1" }
hyper = { version = "0.14.12", default-features = false }
hyper = { version = "0.14.20", default-features = false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the hyper upgrade necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't, I just saw that the server folks were using .20 so I figured I'd update our stuff too. I can remove this if you like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please limit stream length to less than 4.294Gb or run this program on a 64-bit computer architecture.
"#;

impl Stream for SdkBody {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reasoning for not doing this up until now was that AsyncIterator was close to becoming stabilized in std, and this will be obsolete as soon as that happens. This is definitely worth discussing further since it looks like that will take longer due to discussion of generic keywords. Maybe we should create another runtime crate similar to aws-smithy-types-convert that adds in useful conversions to the current set of async traits that are widely used until they become std.

There are a couple of crates that need to get revisited as we get closer to a stable release:

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@Velfi Velfi closed this Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants