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

GrpcServer can't be mounted in a Vert.x Web router #84

Closed
tsegismont opened this issue Dec 22, 2023 · 4 comments
Closed

GrpcServer can't be mounted in a Vert.x Web router #84

tsegismont opened this issue Dec 22, 2023 · 4 comments
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation
Milestone

Comments

@tsegismont
Copy link
Contributor

The documentation and the 4.3 release blog post indicate that a GrpcServer can be mounted in a Vert.x Web router.

In fact, it is not possible because the gRPC server is a Handler<HttpServerRequest> not a Handler<RoutingContext>. So this blog post example is wrong:

GreeterGrpc.GreeterImplBase service = new GreeterGrpc.GreeterImplBase() {
  @Override
  public void sayHello(HelloRequest request, StreamObserver<HelloReply> responseObserver) {
    responseObserver.onNext(HelloReply.newBuilder().setMessage("Hello " + request.getName()).build());
    responseObserver.onCompleted();
  }
};

GrpcServer grpcServer = GrpcServer.server(vertx);
GrpcServiceBridge serverStub = GrpcServiceBridge.bridge(service);
serverStub.bind(grpcServer);

router.consumes("application/grpc").handler(grpcServer); // does not compile
@tsegismont tsegismont added the bug Something isn't working label Dec 22, 2023
@tsegismont tsegismont added this to the 4.5.2 milestone Dec 22, 2023
@tsegismont tsegismont self-assigned this Dec 22, 2023
@tsegismont
Copy link
Contributor Author

@vietj did I miss anything?

tsegismont added a commit to tsegismont/vertx-grpc that referenced this issue Jan 3, 2024
Fixes eclipse-vertx#84

In order to integrate Vert.x Web routes and gRPC servers, we need an adapter.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit to tsegismont/vertx-grpc that referenced this issue Jan 3, 2024
See eclipse-vertx#84

In order to integrate Vert.x Web routes and gRPC servers, we need an adapter.

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
@vietj
Copy link
Member

vietj commented Jan 8, 2024

I would like to avoid this dependency of vertx-grpc over vertx-web for various reasons.

Instead we should provide the code return rc -> server.handle(rc.request()) in the documentation, alternatively we could consider having a dependency of vertx-web onto vertx-grpc, but I think that is not necessary.

@tsegismont
Copy link
Contributor Author

OK, let's fix the documentation and the blog post

@tsegismont tsegismont added the documentation Improvements or additions to documentation label Jan 10, 2024
tsegismont added a commit to tsegismont/vertx-grpc that referenced this issue Jan 11, 2024
See eclipse-vertx#84

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit to tsegismont/vertx-grpc that referenced this issue Jan 11, 2024
See eclipse-vertx#84

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
vietj pushed a commit that referenced this issue Jan 19, 2024
See #84

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
tsegismont added a commit that referenced this issue Jan 19, 2024
See #84

Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
@tsegismont
Copy link
Contributor Author

Fixed by 31d21d8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants