aio-libs-abandoned / aioredis-py

asyncio (PEP 3156) Redis support
https://aioredis.readthedocs.io/
MIT License
2.3k stars 336 forks source link

subscription callback function cannot be coroutine or awaitable #1235

Open dnlwgnd opened 2 years ago

dnlwgnd commented 2 years ago

Describe the bug

I am using aioredis in a FastAPI app. I want to use the pubsub feature and setup subscriptions with callback functions.

Now my callback functions are async in nature and thus cooroutine instead of normal functions because inside them i need to await other coroutines (in my case sending content to a websocket entpoint). Unfortunately the code inside pubsub.run() (which calls get_message() which calls handle_message()) does not check if a callback might be an awaitable and should be awaited instead of called normally.

Instead the coroutine is never awaited, which raises a Warning and of course does not execute code inside the callback.

To Reproduce

setup a pubsub with a callback subscription. The callback is a async def ... publish a message

Expected behavior

The awaitable callback function should be awaited instead of directly called.

Logs/tracebacks

..../lib/python3.10/site-packages/aioredis/client.py:4177: RuntimeWarning: coroutine 'websocket_endpoint.<locals>.send_messages' was never awaited
  handler(message)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Python Version

$ python --version
Python 3.10.0

aioredis Version

$ python -m pip show aioredis
Version 2.0.0

Additional context

No response

Code of Conduct

Andrew-Chen-Wang commented 2 years ago

I can't recall from top of my head, but does order matter when it comes to callbacks? If so, then all passed in callbacks must convert to coroutines which is breaking change. If not, then we can use asyncio.iscoroutinefunction to decide whether to await or not (I believe awaiting does slow down code by a little if no I/O is being done and comparing to a sync function that also does no I/O).

If that didn't make sense, I'm just asking you to file a PR and I'll assign proper reviewers. Thanks.

stephanm commented 2 years ago

This didn't work for my use case, too.

@Andrew-Chen-Wang, maybe I didn't fully understand your concerns, but IMHO the order of the callbacks isn't guaranteed anyway as they are stored in a dict (unordered!) keyed by the channel. Maybe you could have a look and assign a reviewer? Thanks!