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

Remove protoc-reflection dependency on protoc bin #1148

Closed
julienfouilhe opened this issue Nov 14, 2022 · 3 comments
Closed

Remove protoc-reflection dependency on protoc bin #1148

julienfouilhe opened this issue Nov 14, 2022 · 3 comments

Comments

@julienfouilhe
Copy link

julienfouilhe commented Nov 14, 2022

Feature Request

Crates

  • tonic-reflection

Motivation

I tried to add reflection to my gRPC server using tonic-reflection. I do not use tonic-build, but rather precompile proto files with protoc plugins, separately.

But tonic-reflection uses include_proto which seems to be using protoc under the hood.

This means that in order to use this package there needs to be protoc installed.
It's a problem for CI/CD pipelines, building a docker image, etc... All these now need to first install the protoc dependency.

Proposal

Since include_proto is used for a single static file, it seems to me that it would be great to "pre compile" it instead?

Alternatives

  • tonic-reflection could be used as an optional dependency.
  • protoc could be installed in all contexts.
@hexfusion
Copy link

hexfusion commented Nov 14, 2022

Completely agree that protoc should be an optional dependency for tonic in general. For example grpc-go does not rely directly on protoc. If it is needed for testing etc it feels these can be gated by feature flags. I have never contributed to the repo but if we are open to the change I would be interested in trying to help.

@LucioFranco
Copy link
Member

If by pre compile you commit the generated code and make it so end users of the library don't need to have a protoc in their path. Then yes we can make that change in line with how tonic-health does it. You can follow this PR #1065 as an example. TL;DR: We create a test that does the build generation that will check that the proto code is up to date with what is committed so when we publish you don't need to build anything. I believe this should solve your problem.

@tottoto
Copy link
Collaborator

tottoto commented Jun 5, 2024

Resolved in #1151.

@tottoto tottoto closed this as completed Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants