-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: update to independent trailer retrieval #16
base: main
Are you sure you want to change the base?
Conversation
e518332
to
65c6e80
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I agree with WebAssembly/wasi-http#154 (comment) that the return type should be future<option<trailers>>
rather than option<future<trailers>>
.
Yup, just made a comment over there -- that was more an artifact of what the |
54c09c3
to
cbf69ec
Compare
Alright @dicej thanks for the feedback, this is ready now. Going to put off updating the wit completely (removing |
62c8e1f
to
6faa5e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One suggestion: at the top of component-async-tests/http/src/lib.rs
, we could change
async: {
only_imports: [
"wasi:http/[email protected]#[static]body.finish",
"wasi:http/[email protected]#handle",
]
},
to
async: {
only_imports: [
"wasi:http/[email protected]#handle",
]
},
and likewise in the wit_bindgen::generate!
macro invocations in e.g. async_http_echo.rs
since body.finish
no longer needs to be implemented or called async
.
Hey @dicej I'm not sure we can do that -- since we need to create that future in the case there are no trailers -- and for that I need access to the |
508415b
to
a90ef47
Compare
This commit updates `wasi:http/types#body.finish` to avoid resolving optional trailers, but instead returning them for the caller to resolve. This may introduce some efficiency gains in the cases where trailers can be safely ignored, and does avoid the work done in checking whether they are present or not, at least. Signed-off-by: Victor Adossi <[email protected]>
Co-authored-by: Joel Dice <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
8ea2715
to
066bf85
Compare
Good point. Let's leave the host side |
No description provided.