Allow creating a `Channel` outside of an async context

[FHTMitchell]

There is no reason to demand that a Channel is constructed in an async context but currently that is (subtly) the case.

class Channel:
    ...
    def __init__(...):
        ...
        if loop:
            warnings.warn("The loop argument is deprecated and scheduled "
                          "for removal in grpclib 0.5",
                          DeprecationWarning, stacklevel=2)
        self._loop = loop or asyncio.get_event_loop()

This can be found here. That call to asyncio.get_event_loop is deprecated outside of an async runtime. See the python docs.

Now this will be fixed by 0.5 where you promise to drop the support for the loop param and will presumably just always grab the asyncio event loop at the call site (if even needed), but we don't have to wait for a breaking release. I propose we do this in the meantime

class Channel:
     def __init__(...):
         if loop:
              warnings.warn(...)
         self._running_loop = loop
         ...
    @property
    def _loop(self):
         # This should only be called from within an async runtime!
         return self._running_loop or asyncio.get_running_loop()

(I did consider setting the loop also in the async def __aenter__ method (if it's unset) but I doubt it gives any performance benefit).

This prevents the kind of errors as seen here:

In [2]: %autoawait
IPython autoawait is `on`, and set to use `asyncio`

In [3]: import vol.vol_grpc

In [4]: import grpclib.client

In [5]: channel = grpclib.client.Channel('localhost', 50003)  # perfectly valid to construct object outside of async runtime

In [6]: async with channel as ch:
    ...:    client = vol.vol_grpc.VolServiceStub(ch)
    ...:    res = await client.RunVolPipeline(vol.vol_grpc.vol.PyvolRequest())
...
RuntimeError: Task <Task pending coro=<InteractiveShell.run_cell_async() running at .../IPython/core/interactiveshell.py:3257> cb=[_run_until_complete_cb() at .../asyncio/base_events.py:157]> got Future <Future pending cb=[_chain_future.<locals>._call_check_cancel() at .../asyncio/futures.py:351]> attached to a different loop.

This is because the loop when we created the Channel is replaced when starting a new loop (in this case automatically by IPython but the result is the same if we explicitly call asyncio.run). The code suggested before would allow this to work.

I'm happy to do the PR, it's basically just what's above :)

[vmagamedov]

Hello, I totally agree that this can be fixed before 0.5 release and without breaking backward compatibility.

I'm only asking to avoid creation of extra "properties" for simplicity. I suggest you to lazily initialise this property maybe inside async def _create_connection(self): method. Users may not use __aenter__ context manager protocol, it is optional for a Channel.

Looking forward to review and merge your PR.

[jomonson]

Regarding this issue, I suspect there's another issue related. When the gRPC client is run within an asyncio context that is later closed, invoking gRPC methods on that client sends the request to the server, but then it just hangs - ignoring the response. I understand that once the client is created, the relevant loop instance is pinned to it, but it's not apparent from the constructor API. So, as a developer, I'm unaware that there's a state that I should manage that grpclib depends on. I'd expect either the client re-fetches the loop on new requests, or raises an exception upon request.

[realchandan]

Is creating a new gRPC channel for every request a good idea? The way this library currently works is that if I create a channel outside of the async context, it creates an event loop.

[vmagamedov]

@realchandan no, gRPC is designed in a way to use one connection (channel) for many concurrent requests. This library also assumes that you use asyncio.run to run your app, in this case there are no problems dealing with async and non-async contexts, everything is async as your entrypoint is async.

It is a common mistake to use async libraries in sync apps where event loop is running only when there is a need to call some async function. Is this what you're trying to do? Event loop should be running all the time and it should be available in any part of your application.

[realchandan]

Hi @vmagamedov , thank you for your response!

What I wanted to know was:

Whether I should do:

# Approach 1

from grpclib.client import Channel

channel = Channel(host="127.0.0.1", port=8080)

async def main():
    # Use `channel` variable here for making gRPC calls
    pass

if __name__ == "__main__":
    import asyncio

    asyncio.run(main=main())

Or, I should do it like this:

# Approach 2

from grpclib.client import Channel

async def main():
    async with Channel(host="127.0.0.1", port=8080) as channel:
        # Use `channel` variable here for making gRPC calls
        pass

if __name__ == "__main__":
    import asyncio

    asyncio.run(main=main())

In the first approach, I clearly get a Python variable that can be imported and used anywhere in my code. In the second approach, the channel is bound to the context only.

Which one is more efficient? I thought doing the second approach creates a new channel every time. Thus, I asked, "Is creating a new gRPC channel for every request a good idea?"