add test/example for outbound request body streaming #1991
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
itowlson
reviewed
Oct 31, 2023
| // won't necessarily have a chance to send the request body if they haven't started doing so yet (or, if they | ||
| // have started, they might not be able to finish before the connection is closed). See | ||
| // https://github.com/bytecodealliance/wasmtime/issues/7413 for details. | ||
| pub fn take_body_stream(&self) -> impl futures::Stream<Item = Result<Vec<u8>, streams::Error>> { |
There was a problem hiding this comment.
Yes and yes. The API change is necessary because there's no other way to work around bytecodealliance/wasmtime#7413, and I'm not sure when that will be fixed or whether it will be backported to Wasmtime 14 (and if it is, probably not in time for Spin 2.0).
There was a problem hiding this comment.
Update: here's a PR with the fix: bytecodealliance/wasmtime#7426
rylev
approved these changes
Oct 31, 2023
| @@ -461,14 +461,24 @@ impl IncomingResponse { | |||
| /// # Panics | |||
| /// | |||
| /// Panics if the body was already consumed. | |||
| pub fn into_body_stream(self) -> impl futures::Stream<Item = Result<Vec<u8>, streams::Error>> { | |||
| // TODO: This should ideally take ownership of `self` and be called `into_body_stream` (i.e. symmetric with | |||
There was a problem hiding this comment.
Sigh... I think this whole API might need another look - it's hard to get the flow right. Oh well...
dicej
force-pushed
the
outbound-request-streaming-test
branch
from
0219a5d to
bd7f08e
Compare
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
This also removes some documentation from `spin_sdk::http::send` which turned out to be incorrect; you _can_ safely await the returned future prior to dropping the body handle. Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Turns out there's no need to delay `await`ing the future returned from `executor::outgoing_request_send`. Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Signed-off-by: Joel Dice <joel.dice@fermyon.com>
dicej
force-pushed
the
outbound-request-streaming-test
branch
from
bd7f08e to
891493a
Compare
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.