Use aiojobs for background tasks #573
Conversation
Codecov Report
@@ Coverage Diff @@
## master #573 +/- ##
=======================================
Coverage 98.17% 98.17%
=======================================
Files 13 13
Lines 1040 1043 +3
Branches 140 140
=======================================
+ Hits 1021 1024 +3
Misses 15 15
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@Dreamsorcerer, this PR for preparing to new release from this TODO list. |
| loop = asyncio.get_event_loop() | ||
| scheduler = loop.run_until_complete(aiojobs.create_scheduler(pending_limit=0, limit=None)) |
There was a problem hiding this comment.
This won't work correctly. When the user runs their application, it'll be on a different loop.
You'll need to instantiate it inside the class, probably in the __init__() method.
There was a problem hiding this comment.
Although, if this is in the decorator, that may have the same problem. We may need to think on this a little further, maybe we can create it in decorator() if it doesn't already exist...
There was a problem hiding this comment.
This also highlights another issue to me, the scheduler and cache should both have .close() called at the end of the application. But, I don't see any way for a decorator to do that.
I'm starting to think that the decorators should have the Cache instance passed to them or something, so the developer can handle the lifetime of the caches properly.
Turns out this is more complex than I thought...
| @@ -38,6 +38,7 @@ | |||
| 'redis:python_version>="3.8"': ["aioredis>=1.3.0,<2.0"], | |||
| "memcached": ["aiomcache>=0.5.2"], | |||
| "msgpack": ["msgpack>=0.5.5"], | |||
| "aiojobs": ["aiojobs>=1.0.0"], | |||
There was a problem hiding this comment.
This isn't an extra, it's required, so needs to be added to install_requires.
|
Depends on #609. |
What do these changes do?
Preparing for new release.
Use aiojobs for background tasks.
Are there changes in behavior for the user?
nope
Related issue number
Resolvers: 536
Checklist
CHANGESfolder<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.