stepancheg / grpc-rust

Rust implementation of gRPC
MIT License
1.37k stars 124 forks source link

Make GrpcHttpService public #88

Closed polachok closed 7 years ago

polachok commented 7 years ago

Makes it possible to use external event loop and server, like this:

struct GrpcService {};
let s = GrpcService;
let def = WhateverServiceServer::new_service_def(s);
let grpc = Arc::new(GrpcHttpService::new(def));

.. setup listener ...

listener.incoming().for_each(|(sock, addr)|) {
  let conf = httpbis::server_conf::ServerConf::new();
  let (conn, future) = httpbis::server_conn::ServerConnection::new_plain_single_threaded(&handle, sock, conf, grpc.clone());
  handle.spawn(future.map_err(|e| error!("{}", e)));
}
stepancheg commented 7 years ago

While I agree this issue is must be done, although I have three concerns.

  1. Probably you are exposing low level of it (which is inconvenient for users and makes it hard to maintain backward compatibility). Probably, ServerBuilder (or better httpbis::ServerBuilder) should have a field Option<reactor::Handle> (or reactor::Remote))

  2. It is interesting, how other projects based on tokio solve the issue.

  3. Test needs to be included, to avoid accidental breakage.

stepancheg commented 7 years ago

cc https://github.com/stepancheg/rust-http2/issues/13

polachok commented 7 years ago

1.I don't quite understand how to make Server/ServerBuilder work with existing event loop 2.Hyper has Http::bind() which returns new server on a new loop and Http::new().bind_connection(handle: &Handle, io: I, remote_addr: SocketAddr, service: S) to use the same way I propose.

stepancheg commented 7 years ago

I've added ServerBuilder::event_loop option to use external event loop: https://github.com/stepancheg/rust-http2/commit/331841802824b57c4bb8a1b03d75ba796270880a.

If that doesn't work, please reopen the issue.