feat(python): makes RedisConnection's close method async, use aclose when possible
#2559
Conversation
…close redis.asyncio.client.Redis
python/bullmq/flow_producer.py
Outdated
| """ | ||
| Close the flow instance. | ||
| """ | ||
| return self.redisConnection.close() | ||
| return await self.redisConnection.close() |
There was a problem hiding this comment.
Do you need to await if the result is a future if you are going to return it anyway? I wonder because in JS is not needed.
There was a problem hiding this comment.
Thanks for your comment! I thought about it a little bit and I think the best solution is to get rid of the return statement completely. This way it is much clearer that you have to await the closing of the connection.
There was a problem hiding this comment.
Hmm, not sure why it is clearer, for instance, is it possible that the user calls close without awaiting it?
There was a problem hiding this comment.
When you see something like
async def foo():
await bar()
it is immediately clear that you have to run await foo(). On the other hand, if you have a function
def foo():
return bar()
it's not so clear in my opinion.
await future instead of returning it
|
Can this get merged so we get rid of the deprecation warning and proper async closing of workers? |
redis.asyncio.client.Redis.closehas been deprecated in Version5.0.1ofredis. Therefore,RedisConnection.closenow usesredis.asyncio.client.Redis.aclosewhenever possible. To ensure backwards compatibility,closeis still used ifacloseis not available.More importantly,
self.conn.aclose()will be awaited (resulting inRedisConnection.closeneeding to be async). In fact, this fixesRuntimeError: Event loop stopped before Future completed.errors I get in certain situations when trying to gracefully stop a worker by runningworker.close().