Thanks for the gRFC. It's certainly a lot to digest. I also recommend trying to revisit some of the poorly designed APIs and not simply support/replicate everything under the async model.
Without going into the implementation, I have a few key concerns: - The grpc.aio namespace as a user-facing thing that users would import would be terrible. Considering going-forward this will hopefully be the default, we should try making it work under the grpc module as much as possible. I think for the most part we can do that and there aren't that many technical reasons they could not coexist under the same namespace. (RpcMethodHandler could add a new attribute `async` for example, that makes the machinery invoke the handler and assume it's an async implementation, for instance, or server could have a new add_async_rpc_handlers method). On the channel side, the async methods could easily live side-by-side, leaving no reason to have it under a separate namespace/object. - I'd like to echo Kailash's comment as well, which is we probably want to be able to host non-async servicers under an existing async server. As the testing section admits, this should not be too hard to emulate. In fact, it is probably ideal to just build everything on asyncio and simply wait on the future in the sync version by default. - As a user, we have come to learn that writing streaming RPCs with coroutine generator based iterators is super painful today and ideally you want to just call send and receive from the context API. - The gRFC does not address how flow control would interact with async. Please note that proper gRPC support requires these two simultaneously: (1) you should be able to have a pending sending recv and send op simultaneously. (2) gRPC C-core enforces the rule of at most a single pending read op and a single pending write op. Other things of note that are potentially unrelated but can probably be taken away in an opportunity for change: - I don't think we should feel constrained with the current Java-in-Python-abstract-class mess status-quo. Idiomatic Python can simply return a conforming object without necessarily inheriting from abstract classes in __init__.py. - The server-side interceptor API current can modify which path can be called and potentially rewrite it. That feature is detrimental for high performance servers. It would be great to think about it. - The client-side channel subscribe design should support a "subscribe only once" mechanism (instead of subscribe never ending till unsubscribe) so that we can kill the additional polling thread. This matches with the C-core API model better. The gRFC is long, so there are likely additional things, but this should be good to get some brainstorming going. Can we separate the general async model from each API minutia? I don't think we should carry over each API element without revisiting it carefully. I hope we don't rush through this. You don't get many chances to naturally do a clean redesign like this. On Tuesday, July 23, 2019 at 12:06:31 AM UTC+2, Kailash Sethuraman wrote: > > Hooray! > > Some comments: > > "Granularity per channel, per server level". Can you please clarify? Does > this mean that a server cannot have 2 or services, some async and others > sync? If so, what about pre-packaged services like healthcheck? It will > also be a hurdle for easy and gradual adoption of the async API into > servers and organizations. > > Will running functions in executors be fully supported? It is a common > pattern to run sync tasks in executors on the asyncio event loop using > https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_in_executor. > > The executors can be process or thread pools, will both be supported out of > the box? > > grpc.aio.Call functions return asyncio.Future, but > grpc.ServerContext.send_initial_metadata returns a Task object. Could you > please elaborate on the rationale here? > > grpc.aio.channel_ready_future not returning a future, but a coroutine. > This can be surprising. > > asyncio Exceptions ( will they be mapped to gRPC Errors where possible)? > While the RFC states that the RPCError exceptions will be the same, it does > not speak to how > https://docs.python.org/3/library/asyncio-exceptions.html are mapped, if > at all. > > There is a comparison between the current gRPC API and the proposed, but > it might be valuable to do a similar take from the asyncio perspective -- > what are the functions/facilities that will be unavailable with the gRPC > async API and how can the similar actions be performed. Eg: asyncio.* > functions such as gather,shield, wait_for, wait, etc > > - It would be very nice if the gRFC could include the full API, given that > this is a new package, and list functions that will be exported ( ie: added > to __all__) > > Thanks! > > > > > On Mon, Jul 22, 2019 at 1:49 PM 'Lidi Zheng' via grpc.io < > [email protected] <javascript:>> wrote: > >> Hi gRPC users, >> >> This proposal is about adding a new set of Async IO native API to gRPC >> Python, which solves concurrency issues and performance issues for gRPC >> Python. >> The new API will be isolated with current API, so the migration can >> happen gradually. >> >> gRFC: >> https://github.com/lidizheng/proposal/blob/grpc-python-async-api/L58-python-async-api.md >> PR for the gRFC: https://github.com/grpc/proposal/pull/155 >> >> Comments and suggestions are welcomed! >> >> Lidi Zheng >> >> -- >> You received this message because you are subscribed to the Google Groups >> "grpc.io" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] <javascript:>. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/grpc-io/92d5e57f-332a-4e48-bdb2-ccc4d515a5bf%40googlegroups.com >> >> <https://groups.google.com/d/msgid/grpc-io/92d5e57f-332a-4e48-bdb2-ccc4d515a5bf%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> > -- You received this message because you are subscribed to the Google Groups "grpc.io" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/641a8416-63c3-48af-999e-006c6f4f67c1%40googlegroups.com.
