eduardomourar edited PR #6878:
Within the wasmtime CLI, the current default behavior is to only inject the synchronous functions to linkers. This will add a flag called--async
that will inject the asynchronous one instead.
This is required in order to enable back thewasi-http
module because it has been implemented only with async functions. Additionally, it will help the async support code path to be tested as async should be enabled by default in the future.This will enable back the
wasi-http
module within the wasmtime CLI. We will make the bindgen synchronous through the tokio executor available in wasmtime.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar has marked PR #6878 as ready for review.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
pchickey submitted PR review:
With a few changes we can accept this PR, thanks!
Just as a heads up, I know you put a lot of labor into keeping the core-wasm
wasmtime::Linker
version of this code working, including the sync side of the bindings. We'll keep those working for the wasmtime 13 release, but after that we are going to swap the implementation over to use wasmtime-wasi::preview2 streams and pollables, as well as component resource types. When we make that switch, we will deprecate the core-wasm interfaces, since we don't want to maintain both.
pchickey submitted PR review:
With a few changes we can accept this PR, thanks!
Just as a heads up, I know you put a lot of labor into keeping the core-wasm
wasmtime::Linker
version of this code working, including the sync side of the bindings. We'll keep those working for the wasmtime 13 release, but after that we are going to swap the implementation over to use wasmtime-wasi::preview2 streams and pollables, as well as component resource types. When we make that switch, we will deprecate the core-wasm interfaces, since we don't want to maintain both.
pchickey created PR review comment:
this is impressive! nice job
pchickey created PR review comment:
Can you back out this formatting change, and some of the others in this file?
pchickey created PR review comment:
Using wasi-http with the component::Linker will fail because it conflicts with the stream methods in the wasmtime-wasi::preview2 implementation. So, we should instead
bail
in this case.
pchickey created PR review comment:
can you please swap all
println!
in this file totracing::debug!
? I hadn't realized those were still in here.
eduardomourar submitted PR review.
eduardomourar created PR review comment:
that is actually not case because we are taking the implementation from the
wasmtime-wasi
crate when building the bindings: https://github.com/bytecodealliance/wasmtime/blob/75f3da7f029695787146cb89da5c8b3abc761738/crates/wasi-http/src/proxy.rs#L53-L54we should be able to migrate the cli test to a component when the module code is deprecated.
pchickey submitted PR review.
pchickey created PR review comment:
right, but without actually using HostInputStream and HostOutputStream, it will still crash at runtime.
eduardomourar updated PR #6878.
eduardomourar submitted PR review.
eduardomourar created PR review comment:
done
pchickey has enabled auto merge for PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
eduardomourar updated PR #6878.
pchickey has enabled auto merge for PR #6878.
pchickey merged PR #6878.
Last updated: Nov 22 2024 at 17:03 UTC