search

stepancheg / grpc-rust

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

Supporting both sync & async calls. #32

Closed kkimdev closed 7 years ago

kkimdev commented 7 years ago

It looks like Async client can only make async requests and sync client can only make sync clients?

Shouldn't we be able to select sync/async request per call?

stepancheg commented 7 years ago

If have idea how API should look like, please share.

BTW, making sync call via async API is easy:

let async_client = ...
let result = async_client.my_func(param).wait().unwrap();

wait functions just "waits" for response synchronously.

kkimdev commented 7 years ago

If all it takes is .wait() I would vote for just not having a separate sync client. Having a separate sync client doesn't improve ergonomic that much as doing a sync call from the current async client is trivial, then the remaining question is whether restricting a client to all sync calls in some situations is beneficial, i.e., in case where using async calls lead to a bug or undesirable effect. But I can't think of such scenarios...

For me, as a user, I think I'm going to use only async clients from now on since even if I only have sync calls today, someday I might want to do few async calls, then I have to change back to async client anyways, which what just happened to me (I mean, not a big deal at all though).

kkimdev commented 7 years ago

Or, if .wait() for all sync calls is not desirable maybe we can consider something like this?

client.sync.my_func(param).unwrap();
client.async.my_func(param).wait().unwrap();
stepancheg commented 7 years ago

.wait() for async calls is exactly how sync client is implemented.

If you need only non-streaming requests, then it's going to be enough.

However, for streaming requests, synchronous client executes iterator.next() in the cpu pool. It it somewhat harder to do it with async client (you need to either guarantee that next is fast or provide own cpu pool).