eduardomourar opened PR #6195 from eduardomourar:feat/wasi-http-align-component
to bytecodealliance:main
.
eduardomourar updated PR #6195.
rvolosatovs created PR review comment:
Does this really belong in
wasi-http
crate or should it be living alongside the existing https://docs.wasmtime.dev/api/wasi_common/table/struct.Table.html inwasi_common
? Should there be awasi_common::component::table::Table
if this is component-model specific?
rvolosatovs submitted PR review.
rvolosatovs submitted PR review.
rvolosatovs created PR review comment:
rvolosatovs submitted PR review.
rvolosatovs created PR review comment:
Aren't these
clone
s redundant? If you didn't calliter()
you could just move these instead, since the vector is moved into this function, right?
rvolosatovs submitted PR review.
rvolosatovs created PR review comment:
This can be cleaner expressed with iterator operations and collect, but if you want to keep the for loop
for (key, value) in entries { map.insert(key, vec![value]) }
it's just this, right?
rvolosatovs submitted PR review.
rvolosatovs created PR review comment:
How about extracting this to a
let body = req.body().unwrap_or_else(/* ... */)
for clarity?
eduardomourar submitted PR review.
eduardomourar created PR review comment:
yes, when wasmtime supports wasi preview2, the wasi common here will most likely contain the table (following the preview2-prototyping approach). until then, i just copied this over and made some small tweaks.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar edited PR #6195.
eduardomourar has marked PR #6195 as ready for review.
eduardomourar requested alexcrichton for a review on PR #6195.
eduardomourar requested wasmtime-core-reviewers for a review on PR #6195.
eduardomourar edited PR #6195:
In the context of
wasi-http
, this PR will prepare the ground for the migration from the preview2-prototyping repo to wasmtime. A table is being defined to simulate the futureresource
type. Additionally, it will simplify using Component Linker with the wasi-http.<details>
<summary>Usage example with WASI Preview2</summary>use host::{command::wasi, WasiCtx}; use std::{any::Any, pin::Pin}; use wasi_cap_std_sync::WasiCtxBuilder; use wasmtime::{ component::{Component, Linker}, Config, Engine, Store, }; use wasmtime_wasi_http::{common, WasiHttpCtx}; pub struct Ctx { wasi: WasiCtx, http: WasiHttpCtx, } pub fn add_to_linker<T: Send>( l: &mut wasmtime::component::Linker<T>, f: impl (Fn(&mut T) -> &mut WasiCtx) + Copy + Send + Sync + 'static, ) -> anyhow::Result<()> { wasi::wall_clock::add_to_linker(l, f)?; wasi::monotonic_clock::add_to_linker(l, f)?; wasi::timezone::add_to_linker(l, f)?; wasi::instance_monotonic_clock::add_to_linker(l, f)?; wasi::instance_wall_clock::add_to_linker(l, f)?; wasi::filesystem::add_to_linker(l, f)?; wasi::poll::add_to_linker(l, f)?; wasi::random::add_to_linker(l, f)?; wasi::exit::add_to_linker(l, f)?; wasi::environment::add_to_linker(l, f)?; wasi::preopens::add_to_linker(l, f)?; Ok(()) } struct WritePipe {} impl common::OutputStream for WritePipe { fn as_any(&self) -> &dyn Any { self } fn write(&mut self, buf: &[u8]) -> Result<u64, common::Error> { println!("{}", String::from_utf8_lossy(buf).into_owned()); Ok(buf.len().try_into()?) } fn write_zeroes(&mut self, nelem: u64) -> Result<u64, common::Error> { Ok(nelem) } fn readable(&self) -> Result<(), common::Error> { Ok(()) } fn writable(&self) -> Result<(), common::Error> { Ok(()) } } #[tokio::main] async fn main() -> anyhow::Result<()> { let mut wasi = WasiCtxBuilder::new().inherit_stdio().build(); let mut http = WasiHttpCtx::new(); http.set_stdout(Box::new(WritePipe {})); http.set_stderr(Box::new(WritePipe {})); let mut config = Config::new(); config.wasm_backtrace_details(wasmtime::WasmBacktraceDetails::Enable); config.wasm_component_model(true); config.wasm_multi_memory(true); config.async_support(true); let engine = Engine::new(&config)?; let component = Component::from_file(&engine, "component.wasm")?; let mut store = Store::new(&engine, Ctx { wasi, http }); let mut linker = Linker::new(&engine); add_to_linker(&mut linker, |ctx: &mut Ctx| &mut ctx.wasi)?; wasmtime_wasi_http::add_to_component_linker(&mut linker, |ctx: &mut Ctx| &mut ctx.http)?; let (wasi, _instance) = wasi::Command::instantiate_async(&mut store, &component, &linker).await?; let result = wasi .call_main(&mut store, &[]) .await .map_err(anyhow::Error::from)?; if result.is_err() { anyhow::bail!( "command returned with failing exit status: {:?}", result.err().unwrap() ); } Ok(()) }
</details>
eduardomourar edited PR #6195:
In the context of
wasi-http
, this PR will prepare the ground for the migration from the preview2-prototyping repo to wasmtime. A table is being defined to simulate the futureresource
type. Additionally, it will simplify using Component Linker with the wasi-http module.<details>
<summary>Usage example with WASI Preview2</summary>use host::{command::wasi, WasiCtx}; use std::{any::Any, pin::Pin}; use wasi_cap_std_sync::WasiCtxBuilder; use wasmtime::{ component::{Component, Linker}, Config, Engine, Store, }; use wasmtime_wasi_http::{common, WasiHttpCtx}; pub struct Ctx { wasi: WasiCtx, http: WasiHttpCtx, } pub fn add_to_linker<T: Send>( l: &mut wasmtime::component::Linker<T>, f: impl (Fn(&mut T) -> &mut WasiCtx) + Copy + Send + Sync + 'static, ) -> anyhow::Result<()> { wasi::wall_clock::add_to_linker(l, f)?; wasi::monotonic_clock::add_to_linker(l, f)?; wasi::timezone::add_to_linker(l, f)?; wasi::instance_monotonic_clock::add_to_linker(l, f)?; wasi::instance_wall_clock::add_to_linker(l, f)?; wasi::filesystem::add_to_linker(l, f)?; wasi::poll::add_to_linker(l, f)?; wasi::random::add_to_linker(l, f)?; wasi::exit::add_to_linker(l, f)?; wasi::environment::add_to_linker(l, f)?; wasi::preopens::add_to_linker(l, f)?; Ok(()) } struct WritePipe {} impl common::OutputStream for WritePipe { fn as_any(&self) -> &dyn Any { self } fn write(&mut self, buf: &[u8]) -> Result<u64, common::Error> { println!("{}", String::from_utf8_lossy(buf).into_owned()); Ok(buf.len().try_into()?) } fn write_zeroes(&mut self, nelem: u64) -> Result<u64, common::Error> { Ok(nelem) } fn readable(&self) -> Result<(), common::Error> { Ok(()) } fn writable(&self) -> Result<(), common::Error> { Ok(()) } } #[tokio::main] async fn main() -> anyhow::Result<()> { let mut wasi = WasiCtxBuilder::new().inherit_stdio().build(); let mut http = WasiHttpCtx::new(); http.set_stdout(Box::new(WritePipe {})); http.set_stderr(Box::new(WritePipe {})); let mut config = Config::new(); config.wasm_backtrace_details(wasmtime::WasmBacktraceDetails::Enable); config.wasm_component_model(true); config.wasm_multi_memory(true); config.async_support(true); let engine = Engine::new(&config)?; let component = Component::from_file(&engine, "component.wasm")?; let mut store = Store::new(&engine, Ctx { wasi, http }); let mut linker = Linker::new(&engine); add_to_linker(&mut linker, |ctx: &mut Ctx| &mut ctx.wasi)?; wasmtime_wasi_http::add_to_component_linker(&mut linker, |ctx: &mut Ctx| &mut ctx.http)?; let (wasi, _instance) = wasi::Command::instantiate_async(&mut store, &component, &linker).await?; let result = wasi .call_main(&mut store, &[]) .await .map_err(anyhow::Error::from)?; if result.is_err() { anyhow::bail!( "command returned with failing exit status: {:?}", result.err().unwrap() ); } Ok(()) }
</details>
brendandburns submitted PR review.
brendandburns created PR review comment:
remove dead code.
brendandburns submitted PR review.
brendandburns created PR review comment:
Is there a reason for the rename?
eduardomourar submitted PR review.
eduardomourar created PR review comment:
for this piece, we need to choose which errors we want to throw in place of others
eduardomourar submitted PR review.
eduardomourar created PR review comment:
just to follow the pattern in the other modules, and also to show that there is a breaking change to the API
alexcrichton requested pchickey for a review on PR #6195.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar requested elliottt for a review on PR #6195.
eduardomourar updated PR #6195.
eduardomourar requested wasmtime-compiler-reviewers for a review on PR #6195.
eduardomourar edited PR #6195:
In the context of
wasi-http
, this PR will use the shared functionality fromwasmtime-wasi
will prepare the ground for the migration from the preview2-prototyping repo to wasmtime. A table is being defined to simulate the futureresource
type. Additionally, it will simplify using Component Linker with the wasi-http module.<details>
<summary>Usage example with WASI Preview2</summary>use wasmtime::{ component::{Component, Linker}, Config, Engine, Store, }; use wasmtime_wasi::preview2::{ command::Command, Table, WasiCtx, WasiCtxBuilder, WasiView, }; use wasmtime_wasi_http::{WasiHttpCtx, WasiHttpView}; pub struct Ctx { table: Table, wasi: WasiCtx, http: WasiHttpCtx, } impl WasiView for Ctx { fn table(&self) -> &Table { &self.table } fn table_mut(&mut self) -> &mut Table { &mut self.table } fn ctx(&self) -> &WasiCtx { &self.wasi } fn ctx_mut(&mut self) -> &mut WasiCtx { &mut self.wasi } } impl WasiHttpView for Ctx { fn http_ctx(&self) -> &WasiHttpCtx { &self.http } fn http_ctx_mut(&mut self) -> &mut WasiHttpCtx { &mut self.http } } #[tokio::main] async fn main() -> anyhow::Result<()> { let wasi = WasiCtxBuilder::new().inherit_stdio().build(); let http = WasiHttpCtx::new(); let mut config = Config::new(); config.wasm_backtrace_details(wasmtime::WasmBacktraceDetails::Enable); config.wasm_component_model(true); config.async_support(true); let engine = Engine::new(&config)?; let component = Component::from_file(&engine, "component.wasm")?; let mut store = Store::new(&engine, Ctx { table, wasi, http }); let mut linker = Linker::new(&engine); add_to_linker(&mut linker)?; wasmtime_wasi_http::add_to_component_linker(&mut linker)?; let (wasi, _instance) = wasi::Command::instantiate_async(&mut store, &component, &linker).await?; let result = wasi .call_main(&mut store, &[]) .await .map_err(|e| anyhow::anyhow!("wasm failed with {e:?}"))? .map_err(|e| anyhow::anyhow!("command returned with failing exit status {e:?}"))?; Ok(()) }
</details>
eduardomourar requested wasmtime-default-reviewers for a review on PR #6195.
eduardomourar updated PR #6195.
eduardomourar submitted PR review.
eduardomourar created PR review comment:
we are using the shared table as provided by
wasmtime-wasi
.
eduardomourar updated PR #6195.
eduardomourar submitted PR review.
eduardomourar created PR review comment:
@pchickey , here is the part where we have a shared stream for both read and write. i solved by populating the table with both streams and just keeping their ids for later use.
elliottt submitted PR review.
elliottt created PR review comment:
Why remove the closure argument here? (That's what's causing the CI build failures currently.)
eduardomourar edited PR #6195:
In the context of
wasi-http
, this PR will use the shared functionality fromwasmtime-wasi
will prepare the ground for the migration from the preview2-prototyping repo to wasmtime. A table is being defined to simulate the futureresource
type. Additionally, it will simplify using Component Linker with the wasi-http module.<details>
<summary>Usage example with WASI Preview2</summary>use wasmtime::{ component::{Component, Linker}, Config, Engine, Store, }; use wasmtime_wasi::preview2::{ command::Command, Table, WasiCtx, WasiCtxBuilder, WasiView, }; use wasmtime_wasi_http::{WasiHttpCtx, WasiHttpView}; pub struct Ctx { table: Table, wasi: WasiCtx, http: WasiHttpCtx, } impl WasiView for Ctx { fn table(&self) -> &Table { &self.table } fn table_mut(&mut self) -> &mut Table { &mut self.table } fn ctx(&self) -> &WasiCtx { &self.wasi } fn ctx_mut(&mut self) -> &mut WasiCtx { &mut self.wasi } } impl WasiHttpView for Ctx { fn http_ctx(&self) -> &WasiHttpCtx { &self.http } fn http_ctx_mut(&mut self) -> &mut WasiHttpCtx { &mut self.http } } #[tokio::main] async fn main() -> anyhow::Result<()> { let wasi = WasiCtxBuilder::new().inherit_stdio().build(); let http = WasiHttpCtx::new(); let mut config = Config::new(); config.wasm_backtrace_details(wasmtime::WasmBacktraceDetails::Enable); config.wasm_component_model(true); config.async_support(true); let engine = Engine::new(&config)?; let component = Component::from_file(&engine, "component.wasm")?; let mut store = Store::new(&engine, Ctx { table, wasi, http }); let mut linker = Linker::new(&engine); add_to_linker(&mut linker)?; wasmtime_wasi_http::add_to_component_linker(&mut linker)?; let (wasi, _instance) = wasi::Command::instantiate_async(&mut store, &component, &linker).await?; let result = wasi .call_main(&mut store, &[]) .await .map_err(|e| anyhow::anyhow!("wasm failed with {e:?}"))? .map_err(|e| anyhow::anyhow!("command returned with failing exit status {e:?}"))?; Ok(()) }
</details>
Depends on: https://github.com/bytecodealliance/wasmtime/pull/6836.
eduardomourar submitted PR review.
eduardomourar created PR review comment:
it will make more sense when using the
preview2::WasiCtx
as laid out in this pr. the wasmtime cli tests will be fixed then
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
pchickey submitted PR review.
pchickey submitted PR review.
pchickey created PR review comment:
# Ensure wit definitions are in sync: both wasmtime-wasi and wasmtime-wasi-http need their own # copy of the wit definitions so publishing works, but we need to ensure they are identical copies.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
eduardomourar updated PR #6195.
elliottt submitted PR review:
This is looking great! I just had two questions before we merge.
elliottt submitted PR review:
This is looking great! I just had two questions before we merge.
elliottt created PR review comment:
What do you think about renaming this module so that it's not colliding with a keyword? Maybe
types.rs
instead?
elliottt created PR review comment:
Does this need to be uncommented?
pchickey submitted PR review.
pchickey created PR review comment:
Eduardo mentioned this on zulip - we are removing it as a module runtime option in the CLI for now, and will re-enable it as part of the component linker once streams & pollables are aligned with wasmtime-wasi, and https://github.com/bytecodealliance/wasmtime/pull/6836 lands.
The fix for now is to uncomment everything and
bail
on line 769 instead.
pchickey submitted PR review.
pchickey created PR review comment:
We really should do that, but I think it can be done in a follow up PR.
pchickey submitted PR review.
pchickey submitted PR review.
pchickey created PR review comment:
pchickey created PR review comment:
#[cfg(not(feature = "wasi-http"))]
pchickey created PR review comment:
}
pchickey created PR review comment:
{
pchickey created PR review comment:
#[cfg(feature = "wasi-http")]
pchickey created PR review comment:
bail!("wasi-http support will be swapped over to component CLI support soon");
pchickey created PR review comment:
{
pchickey created PR review comment:
pchickey created PR review comment:
}
pchickey updated PR #6195.
pchickey updated PR #6195.
pchickey updated PR #6195.
pchickey updated PR #6195.
pchickey updated PR #6195.
pchickey updated PR #6195.
pchickey updated PR #6195.
pchickey updated PR #6195.
pchickey updated PR #6195.
pchickey submitted PR review.
pchickey has enabled auto merge for PR #6195.
pchickey has disabled auto merge for PR #6195.
eduardomourar updated PR #6195.
elliottt updated PR #6195.
pchickey has enabled auto merge for PR #6195.
elliottt updated PR #6195.
eduardomourar updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt updated PR #6195.
elliottt has enabled auto merge for PR #6195.
elliottt merged PR #6195.
Last updated: Nov 22 2024 at 17:03 UTC