You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "aiguofer (via GitHub)" <gi...@apache.org> on 2023/03/29 18:48:18 UTC

[GitHub] [arrow] aiguofer opened a new issue, #34778: [Java] Ability to register additional grpc services with FlightServer

aiguofer opened a new issue, #34778:
URL: https://github.com/apache/arrow/issues/34778

   ### Describe the enhancement requested
   
   We want to add the [gRPC health endpoint](https://github.com/grpc/grpc-java/blob/master/services/src/generated/main/grpc/io/grpc/health/v1/HealthGrpc.java) to our server. It looks like the ability to do this was added to the [go server with this PR.](https://github.com/apache/arrow/pull/13995/files). It would be great to have something similar for the Java server.
   
   ### Component(s)
   
   Java


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] aiguofer commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489175655

   >  Use FlightGrpcUtil for full control; we are trying not to expose gRPC APIs directly from core APIs.
   
   I've been looking at `FlightServer` and `FlightGrpcUtil`... I'm not sure how `FlightGrpcUtil` is helpful here? Are you saying we should re-implement our own entire `FlightServer` builder to add more services?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] aiguofer commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1491080575

   @lidavidm I've been struggling with generics and how to store the hook from `withTransportHook(Consumer<T> hook)` for later use in `.build()`.
   
   Is it safe to assume in the future all builders will be either `AbstractServerImplBuilder` or `ServerBuilder`? That would make it a lot easier to do something like:
   
   ```
       public Builder withTransportHook(Consumer<ServerBuilder<?>> hook) {
         this.transportHook = hook;
         return this;
       }
   ```
   and in `build()`:
   ```
         transportHook.accept(builder);
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] aiguofer commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489353139

   I believe the Go API does as well. Would the above commit seem like a reasonable proposal? I can make a PR and move the conversation there.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489400375

   @jiezhang would this sort of API solve your concern in #34612?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489341776

   Yes, and FlightServer does not do all that much for you in the first place, so I don't really want it to be a giant wrapper around the gRPC server builders...just use the gRPC ones directly.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489351789

   Of course, the C++/Python APIs expose some generic hooks for this kind of customization in the core API. A proposal for a similar hook in Java would be welcome. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] aiguofer commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489379670

   Gotcha, that makes sense! I'll take a stab at it!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489373337

   Not quite, directly exposing gRPC is specifically what we have tried to avoid. 
   
   I'm thinking something like (directly off the top of my head, not sure if this compiles)
   
   ```java
   <T> Builder withTransportHook(Consumer<T> hook) {
       if (grpcBuilder instanceof T) {
           hook.accept((T) grpcBuilder);
       } else {
           // raise exception
       }
   }
   
   // ...
   
   FlightServer.builder().withTransportHook<NettyServerBuilder>((nsb) -> { nsb.registerService(...); })...;
   ```
   
   (Go and Rust forego flexibility and directly tie themselves to the gRPC API: that means that a project like Flight-UCX is not possible in those languages, not without rewriting your Flight client/service. That's what we are trying to avoid.)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1491081132

   I think that's fine for this implementation. We can refactor when we have other implementations.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489136298

   Use FlightGrpcUtil for full control; we are trying not to expose gRPC APIs directly from core APIs.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] aiguofer commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1491164596

   @lidavidm got an initial PR out. Let me know what you think about the API before I go on making the Unit tests and docs changes. I tested this with our server and I can add both Health Check and Reflection services.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] aiguofer commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489347961

   @lidavidm so `FlightServer` is not the recommended way to set up a Flight server? Maybe it should be deprecated and moved to examples instead. Seems like a simple [change like this](https://github.com/apache/arrow/compare/main...aiguofer:arrow:add_extra_grpc_services_to_flightserver) would add what we need, but if you prefer not supporting `FlightServer` we can just copy-pasta what we need.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm commented on issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #34778:
URL: https://github.com/apache/arrow/issues/34778#issuecomment-1489350189

   It is still recommended, but if you want to control every gRPC knob we can't really support that in the core API. Again, the main constraint is not leaking gRPC implementation details (as best as possible) into the core API since we are still working on supporting non-gRPC transports.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] lidavidm closed issue #34778: [Java] Ability to register additional grpc services with FlightServer

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm closed issue #34778: [Java] Ability to register additional grpc services with FlightServer
URL: https://github.com/apache/arrow/issues/34778


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org