-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow creating a Channel
outside of an async context
#161
Comments
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 Looking forward to review and merge your PR. |
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. |
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. |
@realchandan no, gRPC is designed in a way to use one connection (channel) for many concurrent requests. This library also assumes that you use 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. |
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?" |
There is no reason to demand that a
Channel
is constructed in an async context but currently that is (subtly) the case.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 theasyncio
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(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:
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 callasyncio.run
). The code suggested before would allow this to work.I'm happy to do the PR, it's basically just what's above :)
The text was updated successfully, but these errors were encountered: