Stream: git-wasmtime

Topic: wasmtime / PR #8676 chore: add test to http request witho...


view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:03):

morenol requested elliottt for a review on PR #8676.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:03):

morenol opened PR #8676 from morenol:add-test-to-http-request-without-port to bytecodealliance:main:

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:03):

morenol requested wasmtime-core-reviewers for a review on PR #8676.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:13):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:13):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:13):

elliottt created PR review comment:

Could you add an additional request for a path that will return a 200?

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:13):

alexcrichton commented on PR #8676:

Thanks! To land this though it'll need to pass CI, so mind folding this into https://github.com/bytecodealliance/wasmtime/pull/8671?

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:15):

morenol commented on PR #8676:

Yes, I was just trying to make sure that this fails on main, but it looks like it is failing not in the expected way for me to fail.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:15):

morenol submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:15):

morenol created PR review comment:

Sure, for some reason this is returning 404 in the CI, any idea why?

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:16):

morenol edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 21 2024 at 21:20):

morenol edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 00:34):

iawia002 commented on PR #8676:

Thanks! To land this though it'll need to pass CI, so mind folding this into #8671?

I am back online, @morenol do you mind if I cherry-pick this commit into #8671 and square them into one commit? I hope we can land this fix soon.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 00:39):

iawia002 edited a comment on PR #8676:

Thanks! To land this though it'll need to pass CI, so mind folding this into #8671?

I am back online, @morenol do you mind if I cherry-pick this commit into #8671 and square them into one commit? For the URL without a port, I think it needs #8671 to be merged first. I hope we can land this fix soon.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 00:39):

morenol commented on PR #8676:

I don't mind but I think that this test is not accurate, feel free to check and create a test that could reproduce the current issue

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 01:01):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 01:01):

elliottt created PR review comment:

Ah, my mistake: this is handing that request to the API_PROXY_COMPONENT test-program, located in crates/test-programs/src/bin/api-proxy.rs. That incoming handler will always return a 200, as it runs sanity checks on the state of the request as seen by an incoming handler.

I think you'll want to add a new incoming handler like api-proxy.rs that actually forwards the requests on, returning the result that it gets directly. Alternatively, you could add a new incoming handler that makes the request to example.com and returns that response, for any incoming request that it receives.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 01:33):

morenol updated PR #8676.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 02:41):

morenol updated PR #8676.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 02:47):

morenol closed without merge PR #8676.

view this post on Zulip Wasmtime GitHub notifications bot (May 22 2024 at 02:47):

morenol commented on PR #8676:

Closed since a similar test was added in #8671


Last updated: Dec 23 2024 at 13:07 UTC