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

Use resources in wasi-http #58

Merged
merged 6 commits into from
Oct 17, 2023

Conversation

elliottt
Copy link
Collaborator

@elliottt elliottt commented Oct 6, 2023

Convert all pseudo-resources to full resources, and as such introduces some borrow<..> annotations.

Additionally, this reworks body handling to go through an intermediate resource, rather than expose direct access to the underlying stream for the request/response resource. This intermediate resource allows us to better track when trailers are expected to be written or read.

@elliottt elliottt marked this pull request as ready for review October 6, 2023 22:42
@elliottt elliottt changed the title Update to match the implementation in wasmtime Use resources in wasi-http Oct 16, 2023
Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for doing this! Once feedback is addressed, feel free to merge or ping me to.

wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
/// are ready to be consumed.
subscribe: func() -> pollable;

/// Retrieve reference to trailers, if they are ready.
Copy link
Member

Choose a reason for hiding this comment

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

It looks like you're switching to doc-comments in a few places, but not all. It seems like we should be all-doc-comments or no-doc-comments (which is making me ask once again: why is this a valuable choice?!).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it would be worthwhile to make all comments doc comments: all the comments we write now are meant to document the api interface, instead of remain as notes for future readers.

@elliottt elliottt merged commit 596e961 into WebAssembly:main Oct 17, 2023
1 check passed
@elliottt elliottt deleted the trevor/match-wasmtime branch December 5, 2023 19:04
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.

2 participants