fix: Enforce http2, fix dfx local server not supported by ic-rosetta-api#7
fix: Enforce http2, fix dfx local server not supported by ic-rosetta-api#7TerrorJack wants to merge 1 commit intodfinity:mainfrom
Conversation
| .build()?; | ||
| runtime.block_on(async { | ||
| let server = Server::bind(&opts.address).serve(service); | ||
| let server = Server::bind(&opts.address).http2_only(true).serve(service); |
There was a problem hiding this comment.
Weird, the doc claims that http2 is supported by default:
A listening HTTP server that accepts connections in both HTTP1 and HTTP2 by default.
-- https://docs.rs/hyper/0.14.13/hyper/server/struct.Server.html
I think strictly requiring HTTP2 here might break quite a few clients
There was a problem hiding this comment.
Instead of directly setting this to true, can you add a new field to Opts, which defaults to false to keep the current default behavior?
There was a problem hiding this comment.
The problem here is icx-proxy is often invoked indirectly by dfx, so we don't get to control the command line options passed to it. Would you accept an environment variable configuration to turn this on?
There was a problem hiding this comment.
Can't we just change all the places where dfx runs icx-proxy (i.e. add the flag to those calls)?
There was a problem hiding this comment.
Adding a command-line option to icx-proxy, and adding parameters to dfx start and dfx bootstrap to control it, sounds like a better option.
|
@TerrorJack, I believe you need to change the PR title to conform to the guideline ( |
Daniel-Bloom-dfinity
changed the title
Daniel-Bloom-dfinity
force-pushed
the
main
branch
21 times, most recently
from
6b5ac96 to
5e94d44
Compare
3 participants
When
dfxstarts a local replica, it automatically startsicx-proxyto reroute the incoming requests. It useshyperfor http logic, andhyperdoesn't properly support http1/http2 dual stack. The mainicrepo assumes http2 in a lot of places, one consequence of that isic-rosetta-apibeing unable to connect to the local replica started bydfx. This patch fixes that problem.See ticket https://dfinity.atlassian.net/browse/ROSETTA1-162 for more context.