Stream: git-wasmtime

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

Wasmtime GitHub notifications bot (May 21 2024 at 21:03):

morenol requested elliottt for a review on PR #8676.

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:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

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
-->

Wasmtime GitHub notifications bot (May 21 2024 at 21:03):

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

Wasmtime GitHub notifications bot (May 21 2024 at 21:13):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (May 21 2024 at 21:13):

elliottt submitted PR review.

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?

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?

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.

Wasmtime GitHub notifications bot (May 21 2024 at 21:15):

morenol submitted PR review.

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?

Wasmtime GitHub notifications bot (May 21 2024 at 21:16):

morenol edited PR review comment.

Wasmtime GitHub notifications bot (May 21 2024 at 21:20):

morenol edited PR review comment.

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.

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.

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

Wasmtime GitHub notifications bot (May 22 2024 at 01:01):

elliottt submitted PR review.

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.

Wasmtime GitHub notifications bot (May 22 2024 at 01:33):

morenol updated PR #8676.

Wasmtime GitHub notifications bot (May 22 2024 at 02:41):

morenol updated PR #8676.

Wasmtime GitHub notifications bot (May 22 2024 at 02:47):

morenol closed without merge PR #8676.

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: Nov 22 2024 at 16:03 UTC