-
Notifications
You must be signed in to change notification settings - Fork 772
Support header-based routing #4036
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
Conversation
50b571e
to
1749e21
Compare
api/server/server.go
Outdated
@@ -163,7 +150,7 @@ func (s *server) Dispatch() error { | |||
|
|||
func (s *server) RegisterChain(chainName string, ctx *snow.ConsensusContext, vm common.VM) { | |||
ctx.Lock.Lock() | |||
handlers, err := vm.CreateHandlers(context.TODO()) | |||
handler, err := vm.NewHTTPHandler(context.TODO()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also did a rename, which I can also undo. But since we're already messing with this interface signature it seemed forgivable to try to sneak in.
- name: go-grpc | ||
out: pb | ||
opt: paths=source_relative | ||
- plugin: connect-go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to compile our connectrpc stuff with this plugin - so for now I think separate buf modules are the cleanest way to do this. I think ideally it's all under proto
, so we'd have a dir structure like proto/connect
and proto/everythingelse
, but if we do a move in this PR it's going to make the diff huge so I made them both top-level dirs for this PR and I think we should do the move under a single proto
dir in a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up issue: #4066
344bf5a
to
f674cc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flake and e2e changes lgtm
d7153f7
to
5d62147
Compare
5d62147
to
66592c7
Compare
Signed-off-by: Joshua Kim <[email protected]>
66592c7
to
0fc4e2c
Compare
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]> Signed-off-by: Joshua Kim <[email protected]>
Why this should be merged
We currently decouple our routers across protocols, but this doesn't support connectrpc which is agnostic to protocols, and wants to handle h1 + h2.
How this works
How this was tested
CI passes
Need to be documented in RELEASES.md?
Yes