Skip to content
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

tonic-health: expose client #471

Closed
LukeMathWalker opened this issue Oct 1, 2020 · 6 comments · Fixed by #602
Closed

tonic-health: expose client #471

LukeMathWalker opened this issue Oct 1, 2020 · 6 comments · Fixed by #602
Labels
A-health C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@LukeMathWalker
Copy link
Contributor

LukeMathWalker commented Oct 1, 2020

Feature Request

Crates

tonic-health

Motivation

Testing a server that exposes the health check service now requires:

  • embedding the protos in your own project;
  • adding a build script to generate the client.

Proposal

Add a new feature flag, client, to tonic-health to expose a gRPC client for the health check service.
Would you prefer to expose the "raw" client as it comes out of tonic-build or would you prefer to wrap it in a nice Rust API that does not expose the underlying protobuf types?

@LucioFranco
Copy link
Member

iirc tonic-health was just for the server side, but I am not opposed to exposing a client. I don't have much time right now to get to this but would love a PR :)

cc @jen20

@LucioFranco LucioFranco added A-health C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue. labels Oct 7, 2020
@LukeMathWalker
Copy link
Contributor Author

A PR you shall have 🎸
Shall we go for raw protobuf types or "wrapped" up nicely in a Rust-ier shell?

@LucioFranco
Copy link
Member

We could expose the pb since I doubt it will change. Just put it into its own module. I think we can then provide a transport feature that enables the connect ability? Not sure tbh

@jen20
Copy link
Contributor

jen20 commented Oct 13, 2020

I definitely planned to get to a client at some point! A PR would be great @LukeMathWalker! Personally I'd tend towards exposing a more idiomatic Rust API if possible, though the PB-generated code is not too bad in this case.

@cortopy
Copy link
Contributor

cortopy commented Apr 9, 2021

It would be great to have the client generated for health checks. Kubernetes does not support grpc health checks natively and requires either a command line tool or a side container that acts as http/grpc proxy for health checks

@LucioFranco
Copy link
Member

Yeah, I agree, we should expose the client and also the proto module (which would include the client).

TransientError pushed a commit to TransientError/tonic that referenced this issue Apr 12, 2021
TransientError pushed a commit to TransientError/tonic that referenced this issue Apr 12, 2021
TransientError pushed a commit to TransientError/tonic that referenced this issue Apr 12, 2021
TransientError pushed a commit to TransientError/tonic that referenced this issue Apr 12, 2021
LucioFranco added a commit that referenced this issue Apr 13, 2021
Co-authored-by: kvwu <kvwu@amazon.com>
Co-authored-by: Lucio Franco <luciofranco14@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-health C-enhancement Category: New feature or request E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants