Wasmtime GitHub notifications bot (Sep 26 2023 at 17:49): Wasmtime GitHub notifications bot (Sep 26 2023 at 17:49):pchickey opened PR #7091 from bytecodealliance:trevor/wasi-http-server to bytecodealliance:main:
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Sep 26 2023 at 17:53):pchickey updated PR #7091.
Wasmtime GitHub notifications bot (Sep 26 2023 at 21:01):pchickey updated PR #7091.
Wasmtime GitHub notifications bot (Sep 26 2023 at 21:05):pchickey updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 17:38):pchickey updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 17:55):pchickey updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 17:59):pchickey updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 18:19):pchickey updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 18:19):pchickey has marked PR #7091 as ready for review.
Wasmtime GitHub notifications bot (Sep 27 2023 at 18:19):pchickey requested alexcrichton for a review on PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 18:19):pchickey requested wasmtime-core-reviewers for a review on PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 18:19):pchickey requested wasmtime-default-reviewers for a review on PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 18:24):pchickey edited PR #7091:
This follows up #7056 with the remaining pieces used for the wasi-http proxy world.
We made a tweak to the wasi-http wit definition which was missed in the prev PR, but wasn't caught because the incoming request side was still stubbed out.
A new test suite wasi-http-proxy-tests has one trivial test for this new functionality.
A new cli subcommand
wasmtime serve [--addr SOCKADDR] proxy_component.wasmruns components in the wasi-http proxy world.We make some additional filesystem and cli imports to the proxy world in both the test runner and cli subdommand because the component adapter pulls them in unconditionally. We will work on fixing that in the adapter in the future.
Wasmtime GitHub notifications bot (Sep 27 2023 at 20:47):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 20:53):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 20:53):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 20:55):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 20:57):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 21:32):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 22:07):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 22:17):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 27 2023 at 22:49):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 28 2023 at 00:16):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 28 2023 at 00:17):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 28 2023 at 00:23):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:30):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton submitted PR review:
Looks great to me :+1:
Small comments here and there, please feel free to defer whatever you like to either issues or future PRs.
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton submitted PR review:
Looks great to me :+1:
Small comments here and there, please feel free to defer whatever you like to either issues or future PRs.
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton created PR review comment:
While you're at it mind putting
=constraints on these as well? (works around issues with Cargo's handling of pre-releases)
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton created PR review comment:
Another possibility here, if you're interested, is to model this as subcommands-as-features. The
wasm-toolsCLI does this where there's a Cargo feature for each subcommand and they're all enabled by default. That way this could be enabled withcargo build --features serveandservewould implicitly enable the component model andwasi-http
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton created PR review comment:
Out of curiosity is it easy enough to have a graceful shutdown here where ctrl-c sets a flag "please exit ASAP"? Otherwise this looks like it cancel all pending work if it's happening during ctrl-c (in which case this probably isn't much better than avoiding installing a ctrl-c in the first place)
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton created PR review comment:
I think it'd be best if this had similar options to
wasmtime run, for example--allow-precompiled,--dir,--env, and--profile. That being said I think it's also totally ok for this to be a follow-up PR too.One thing that I think should also be done is to share the code that configures WASI, e.g. processing many of the
-Soptions. For example in theorywasi-nnand such should all be possible to enable too. That would require some refactoring, however, so perhaps best to open an issue and/or file a follow-up PR for that.
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton created PR review comment:
This can all be moved outside of the enclosing
async moveI think and theFuturereturn could actually be just the receiver (or a boxed receiver, either way)
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton created PR review comment:
I think this can probably be dropped because the task doesn't return
Err(_)anywhere
Wasmtime GitHub notifications bot (Sep 28 2023 at 18:40):alexcrichton created PR review comment:
I'll leave the decision up to y'all though, I don't feel strongly either way
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:11):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:11):elliottt created PR review comment:
I like that!
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:12):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:12):elliottt created PR review comment:
Actually,
wasi-httpalready enablescomponent-model, so we could make this only depend onwasi-http. However, I don't like that as much because it's not quite as declarative as theservecommand feature.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:17):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:17):elliottt created PR review comment:
Yep, agreed. I'll do that in a follow-up PR.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:18):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:18):elliottt created PR review comment:
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:19):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:19):elliottt created PR review comment:
Yeah, I think that should be possible: we could share some state with the running thread that we check for each iteration of the accept loop.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:35):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:35):elliottt created PR review comment:
I'm going to punt this to a follow-up PR, in the interest of landing this so that the resourcification work can proceed.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:51):elliottt updated PR #7091.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:53):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Sep 28 2023 at 19:53):elliottt created PR review comment:
Wasmtime GitHub notifications bot (Sep 28 2023 at 20:51):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Sep 28 2023 at 20:51):elliottt created PR review comment:
Great idea! I like that this makes it so much more clear that we spawn the task and return the future that waits on the oneshot channel, rather than returning a future that does both.
Wasmtime GitHub notifications bot (Sep 28 2023 at 20:55):elliottt has enabled auto merge for PR #7091.
Wasmtime GitHub notifications bot (Sep 28 2023 at 23:42):elliottt merged PR #7091.
Last updated: Nov 22 2024 at 16:03 UTC