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

gRPC reflection v1 support #115

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

gRPC reflection v1 support #115

wants to merge 2 commits into from

Conversation

wvuong
Copy link

@wvuong wvuong commented Sep 4, 2024

Motivation:

Add support for the gRPC reflection api which is commonly used by gRPC tooling like Postman, Insomnia, k6, grpcurl for service discovery and transcoding. Fixes #22

  • Added the v1 reflection.proto from gRPC
  • Updated pom.xml to compile reflection.proto
  • Added GrpcServerIndex which contains service descriptors
  • Added ReflectionServiceV1Handler which is a Vertx gRPC handler for handling reflection requests
  • Added ReflectionServiceV1Test to test and demonstrate usage

Conformance:

Eclipse Contributor Agreement signed.

@vietj
Copy link
Member

vietj commented Sep 4, 2024

can we try to make this work for vertx-grpc-server as well ?

@wvuong
Copy link
Author

wvuong commented Sep 5, 2024

Yes, we have that working on the 4.x branch at my job. However for 5.x, GrpcServerIndex is dependent on a few classes from io.grpc:grpc-api to parse out the service descriptors. And it looks like you are trying to keep a separation and don't want to add io.grpc:grpc-api as a direct dependency in vertx-grpc-server...

Perhaps this is better as a new submodule in the project so that people can use it with vertx-grpc-server or vertx-grpcio-server?

@vietj
Copy link
Member

vietj commented Sep 5, 2024

which API is necessary ? vertx has its own generator so perhaps we can have this working as well using the generator.

@vietj
Copy link
Member

vietj commented Sep 5, 2024

I think I will merge it as is and take it from here as I need it with pure gRPC to build JPMS/gRPC example

@wvuong
Copy link
Author

wvuong commented Sep 5, 2024

for GrpcServerIndex

import io.grpc.ServerServiceDefinition;
import io.grpc.ServiceDescriptor;

from io.grpc:grpc-api

@vietj
Copy link
Member

vietj commented Sep 5, 2024

I think actually for now what we are missing is documentation, can you add this ? it is very important

@wvuong
Copy link
Author

wvuong commented Sep 5, 2024

I can update ioserver.adoc and add an example tonight

@vietj
Copy link
Member

vietj commented Sep 9, 2024

thanks @wvuong

@vietj vietj added this to the 5.0.0 milestone Nov 14, 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

Successfully merging this pull request may close these issues.

ProtoReflectionService "server" is null exception
2 participants