-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
Fixes #32076 -- Add async cache methods to Base, Dummy, and LocMem Cache #13508
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
Conversation
* Based on DEP 0009 and the implementation found [here](https://github.com/Andrew-Chen-Wang/django-async-redis) * Copied all doc strings from the synchronous methods
So as I'm implementing LocMem (I think I want to add this in a different ticket), I'm writing the async test cases right next to their synchronous versions. An issue arising is if the currently tested backend doesn't have the Let me know if LocMem should be implemented in this ticket or in a separate ticket btw. Test cases are mostly passing... just a couple of sync_to_async methods failing. Edit: I finished writing the LocMem tests. Default use sync_to_async |
* Per DEP 9, thanks to @felixxm, the async methods of the cache default to running the synchronous version. * Accidentally closed the cache in decr_version_async Signed-off-by: Andrew-Chen-Wang <[email protected]>
* Locks implement asyncio.Lock if in an async context * All BaseTest cases for cache were copied from their synchronous versions * Added async LocMem specific test cases Extra Notes: * test_cache_write_unpicklable_object_async requires the Middleware mixin to be asynchronous. Thus I have left a TODO in L1825 * test_per_thread_async creates a new event loop for each new thread in order to test with async locks Signed-off-by: Andrew-Chen-Wang <[email protected]>
* Some test cases needed to be overridden in the Memcached specific test cases. The async versions of those sync overriding test cases are now implemented Signed-off-by: Andrew-Chen-Wang <[email protected]>
Oh yeah... need to support python 3.6/7.... So the problem was AsyncMock was added in Python 3.8... is there any AsyncMock implemented in Django's tests is the question... |
* Had to add AsyncMock support for Python 3.6-7 support in test_get_or_set_racing_async * Also linted with flake8 for code that wasn't linted before? Signed-off-by: Andrew-Chen-Wang <[email protected]>
Sigh... one Python version, one memcached library, one async test case with no sync version that was overriding... I'm not even sure how to fix this one... |
Please don't change a scope of accepted tickets, see comment. |
New PR instead of reverting |
ticket-32076
Issues I currently see are:
django.core.cache
(init file).Lots of times in several areas in Django, the cache is called by doingif "string" in cache
. The async method cannot handle the magic method__contains__
unless you usesync_to_async
. I dislike context switching, so I would rather not touch that, but I can see why people would want to "accidentally" use it, even if in the documentation we mention it will not be performant and people should strive to useawait cache.has_key("string")
instead.The issue I see with that is if a library accidentally doesn't implement the get_async() method, then the error will say that the
get
method is not created when it should really say the dev needs to create theget_async
method.4. I'm not going to have time to write documentation (Edit rephrased: I'm terrible at documentation and heavily discouraged).
5. I've added LocMem to this PR. The only change is that we're adding asyncio.Lock for async context. I think I can rework this to take advantage of the RunTimeError so we know which context we're in. That means we don't need to create two different locks.
If you'd like, I can add my setup for LocMem backend. I'm not too sure about the tests though. As I was copying and pasting all those test cases, my computer started burning up...