-
Notifications
You must be signed in to change notification settings - Fork 49
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 a gRPC connector to backfila #398
Conversation
853a65f
to
e4e786a
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.
I think we don't want the Backfila grpc service
?
Otherwise I would think we would need to do more to the guice config to configure it properly?
Also what about the path that GRPC would use? Or does that no longer matter?
@@ -104,3 +104,10 @@ message CheckBackfillStatusResponse { | |||
COMPLETE = 3; | |||
} | |||
} | |||
|
|||
service BackfilaService { |
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.
I'm not sure we need this unless we want people to use GRPC to talk to Backfila as well.
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.
Yeah, I debated whether or not we should add this from a symmetry perspective. Don't feel strongly either way, but it would be nice to support gRPC bidirectionally in the future (since we standardized on it internally).
def3a3c
to
617a7dc
Compare
Yeah, this is more aspirational currently. Not 100% sure how to wire up an additional gRPC variant in Misk.
gRPC doesn't need paths (the path is effectively specified by the package name), we just need to specify the authority for the client. |
No description provided.