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

add support for wasi:http/incoming-handler@0.2.0-rc-2023-11-10 #2166

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

dicej
Copy link
Contributor

@dicej dicej commented Dec 13, 2023

This builds upon Alex's excellent work in
#2108, adding the ability to run guests which target the 0.2.0-rc-2023-11-10 version of wasi-http.

I've added a test which uses wit-bindgen and wit-component directly to ensure it uses the correct snapshot for everything:wasi-http, wasi-cli, etc.

Note that we're not yet updating any SDKs to use the new snapshot, since we still want people to be able to use the new SDK to target Spin 2.0.

This builds upon Alex's excellent work in
spinframework#2108, adding the ability to run guests
which target the `0.2.0-rc-2023-11-10` version of `wasi-http`.

I've added a test which uses `wit-bindgen` and `wit-component` directly to
ensure it uses the correct snapshot for everything `wasi-http`, `wasi-cli`, etc.

Note that we're not yet updating any SDKs to use the new snapshot, since we
still want people to be able to use the new SDK to target Spin 2.0.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
@dicej dicej requested review from alexcrichton and lann December 13, 2023 19:21
…ponent

Previously, I tried to be clever and overwrite the module with the component,
but that led to a filesystem-level race condition and was just generally
fragile.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
@dicej dicej merged commit 9b2c657 into spinframework:main Dec 13, 2023
@dicej dicej deleted the rc-2023-11-10 branch December 13, 2023 21:27
Copy link
Collaborator

@rylev rylev left a comment

Choose a reason for hiding this comment

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

Is there any reason that the tests aren't runtime tests? If not, I can submit a PR to convert them.

@dicej
Copy link
Contributor Author

dicej commented Dec 14, 2023

Is there any reason that the tests aren't runtime tests? If not, I can submit a PR to convert them.

I try to avoid Docker when testing and debugging since it adds overhead and (IMO) not a lot of value for simple tests like this. Can (a subset of) the runtime tests be run without it?

@rylev
Copy link
Collaborator

rylev commented Dec 14, 2023

@dicej we only run inside of Docker in CI currently because that's where the e2e tests run - the runtime tests have no dependency on Docker and can be run outside of a Docker container (which I do locally). Avoiding one-off tests like this is the exact goal of the runtime tests, so if you're feeling resistance towards adding one, it makes me want to have a conversation again about how we unify testing.

@dicej
Copy link
Contributor Author

dicej commented Dec 14, 2023

I didn't realize Docker was optional -- glad to hear it is. Maybe we can pair up and you can walk me through converting this to a runtime test?

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.

3 participants