Skip to content
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

wasi-http: Allow embedder to manage outgoing connections #7288

Merged

Conversation

dicej
Copy link
Contributor

@dicej dicej commented Oct 18, 2023

This adds a new send_request method to WasiHttpView, allowing embedders to override the default implementation with their own if they desire. The default implementation behaves exactly as before.

I've also added a few new wasi-http tests: one to test the above, and two others to test streaming and concurrency. These tests are ports of the test_wasi_http_echo and test_wasi_http_hash_all tests in the Spin integration test suite. The component they instantiate is likewise ported from the Spin
wasi-http-rust-streaming-outgoing-body component.

Fixes #7259

@dicej dicej requested a review from elliottt October 18, 2023 20:19
@dicej dicej requested review from a team as code owners October 18, 2023 20:19
@dicej dicej requested review from alexcrichton and removed request for a team October 18, 2023 20:19
@dicej
Copy link
Contributor Author

dicej commented Oct 18, 2023

I was wondering if cargo-vet would complain... and it does. I thought maybe adding dependencies to test-only code would be okay, but clearly not. I'll see what I can do to address it.

@dicej dicej force-pushed the wasi-http-override-send-request branch 2 times, most recently from c1f982e to 605d839 Compare October 18, 2023 21:05
This adds a new `send_request` method to `WasiHttpView`, allowing embedders to
override the default implementation with their own if the desire.  The default
implementation behaves exactly as before.

I've also added a few new `wasi-http` tests: one to test the above, and two
others to test streaming and concurrency.  These tests are ports of the
`test_wasi_http_echo` and `test_wasi_http_hash_all` tests in the
[Spin](https://github.com/fermyon/spin) integration test suite.  The component
they instantiate is likewise ported from the Spin
`wasi-http-rust-streaming-outgoing-body` component.

Fixes bytecodealliance#7259

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
@dicej dicej force-pushed the wasi-http-override-send-request branch from 605d839 to 7deaae4 Compare October 18, 2023 21:23
Copy link
Member

@elliottt elliottt left a comment

Choose a reason for hiding this comment

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

This looks great to me, thank you!

Comment on lines +59 to +62
fn send_request(
&mut self,
request: OutgoingRequest,
) -> wasmtime::Result<Resource<HostFutureIncomingResponse>>
Copy link
Member

Choose a reason for hiding this comment

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

This is so much simpler than what I was assuming we'd need, nice work!

@elliottt elliottt added this pull request to the merge queue Oct 18, 2023
@pchickey
Copy link
Contributor

Yes, thank you Joel, I really like how the design of this worked out.

Merged via the queue into bytecodealliance:main with commit cc3bf3c Oct 18, 2023
18 checks passed
dicej added a commit to dicej/wasmtime that referenced this pull request Oct 19, 2023
This is a backport of bytecodealliance#7288,
minus the tests.  The test suite has been refactored since the release-14.0.0
branch was created, so backporting the tests would be complicated and not worth
the effort.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
alexcrichton pushed a commit that referenced this pull request Oct 19, 2023
This is a backport of #7288,
minus the tests.  The test suite has been refactored since the release-14.0.0
branch was created, so backporting the tests would be complicated and not worth
the effort.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
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.

wasi-http: Manage outgoing connections with a callback on WasiHttpCtx
3 participants