Remove unneeded loop arguments
#1006
Conversation
This makes aioredis more consistent with other libraries that have removed the need to explicitly specify the loop to use. This has two knock-on effects: 1. __del__ will now always use asyncio.get_event_loop(). But `__del__` is broken anyway, for reasons I've explained in aio-libs-abandoned#930. 2. PubSub.run_in_thread is a synchronous function but needs to know the event loop to run. I've updated it to take an explicit loop argument. This makes it possible to construct the Redis object before the event loop. I'm going to file a separate issue to remove this function anyway, since aioredis should provide concurrency via asyncio tasks rather than threads, and the one test function has been disabled because it is broken. Closes aio-libs-abandoned#1002.
|
I'm not sure what to do with regards to testing around PubSub.run_in_thread. redis-py has very minimal testing of this function (just one test for a particular feature), and @seandstewart disabled it in #891 with a comment that it was broken and made the tests hang. |
Codecov Report
@@ Coverage Diff @@
## master #1006 +/- ##
==========================================
- Coverage 88.62% 88.61% -0.01%
==========================================
Files 21 21
Lines 6784 6781 -3
Branches 653 653
==========================================
- Hits 6012 6009 -3
Misses 610 610
Partials 162 162
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Thanks for the work here - it was on my docket to get rid of these so I'm glad it's getting done! |
What do these changes do?
The new 2.0 implementation has a number of places where it either takes an explicit loop parameter, or passes one to an asyncio function. The need for this was largely removed in Python 3.5.3 (because get_event_loop was changed to return the event loop of the current task when called from inside a task), and many functions (such as wait_for deprecated the parameter in 3.8 with removal for 3.10. Other functions (like open_connection) didn't deprecate it, but made it optional and documented that it is not needed when called from a coroutine. I wouldn't be surprised if they are deprecated in future.
Are there changes in behavior for the user?
Only compared to 2.0.0a1:
__del__will now always use asyncio.get_event_loop(). But__del__is broken anyway, for reasons I've explained in Migration to aioredis 2.0 #930.PubSub.run_in_thread is a synchronous function but needs to know the event loop to run. I've updated it to take an explicit loop argument. This makes it possible to construct the Redis object before the event loop. I'm going to file a separate issue to remove this function anyway, since aioredis should provide concurrency via asyncio tasks rather than threads, and the one test function has been disabled because it is broken.
Related issue number
#1002
Checklist
CONTRIBUTORS.txt<Name> <Surname>.CHANGES/folder<issue_id>.<type>(e.g.588.bugfix)issue_idchange it to the pr id after creating the PR.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.Fix issue with non-ascii contents in doctest text files.