aiocache
aiocache copied to clipboard
[Bugfix] Add redis-py >= 4.2.0 support
2022.05.12: switched backend from aioredisto
redis-py >= 4.2.0`.
Update:
- To test with
aioredis
2.0.0 automatically. Travis and tox configurations need to be updated. - Fixes aio-libs/aiocache#543
What do these changes do?
aioredis 2.0.0 released with breaking API change. Update current backend implementation of airedis/backends/redis.py
.
Breaking aioredis changes encounter during making this PR. aio-libs/aioredis-py#1109
Are there changes in behavior for the user?
Nope. Lower level redis backend change. No public API affected.
Related issue number
Checklist
- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [ ] Documentation reflects the changes
- [ ] Add a new news fragment into the
CHANGES
folder- name it
<issue_id>.<type>
(e.g.588.bugfix
) - if you don't have an
issue_id
change it to the pr id after creating the PR - ensure type is one of the following:
-
.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.
-
- Make sure to use full sentences with correct case and punctuation, for example:
Fix issue with non-ascii contents in doctest text files.
- name it
hi @laggardkernel the PR LGTM! I like it! 👍 let me know if you want me to help on anything missing from the list! thanks a ton
Would love to see this merged
@laggardkernel Thank you for your efforts! It's a really wanted feature.
Is this project still maintained? How can we as a group go about getting this merged? For us, this is blocking the migration of some keen souls to use Python 3.10 in the company.
Is this project still maintained? How can we as a group go about getting this merged? For us, this is blocking the migration of some keen souls to use Python 3.10 in the company.
You can create a fork from this project and make any adjustments you want. It's pretty easy. If you need to download the package from pip, please upload it to pypi.
We may indeed have to go down that route if there is no chance of this change getting into the codebase any time soon -- is this the case? The changes here seem functionally complete?
In other words, is it 'official' that this codebase is no longer maintained?
Hi, sadly I'm not maintaining this project anymore but I'm happy to invite people to the project in case anyone wants to volunteer.
@argaen has anybody volunteerd as of yet?
If anyone wants to make a start, feel free to start PRs for migrating the CI to Github. See: https://github.com/aio-libs/aiocache/issues/536#issuecomment-1024448383
I think keeping up the multi-version support is unsustainable and has little benefit. I'd like to just set the minimum version to aioredis>=2
for the next release and keep things simple. If someone has time to update this PR to only use v2, that would be great. Otherwise, I'll look at updating later.
@Dreamsorcerer Okay, I'd like to have a try to adapt the PR with only aioredis>=2
support when I have spare time.
@laggardkernel Did you get blocked on something, or just didn't get around to this? Otherwise, me (or someone else) could try to pick this up.
@Dreamsorcerer Just noticed that aioredis-py suggests using the "original" redis-py instead
Aioredis is now in redis-py 4.2.0rc1+
Would you agree it makes more sense to refactor aiocache to use redis-py than to make it ready for aioredis-py 2.x when the latter is likely to be deprecated?
Yes, might as well move to that library then. Though it's the same work, it looks like they've merged aioredis upstream, so the only difference currently is the import statement.
WIP: Updated the PR with only aioredis 2.x support. The test is fixed and passed, but the pytest fixtures may need some updates. I'll check that later.
Would you agree it makes more sense to refactor aiocache to use redis-py than to make it ready for aioredis-py 2.x when the latter is likely to be deprecated?
I have read both aioredis-py
and redis-py
before. The last version of redis-py
I read is still 4.1. I'll check the latest release of redis-py soon.
@Mark90 The time when I opened the PR I was unemployed. I had a good time browsing and reading all kinds of Python libraries, learning how they are designed. Now i'm back to work and kind of busy. Sorry. 🙇♂️
aioredis 2.x only support is done.
This PR is firstly made with backward compatiblity taken into consideration. It supported for both aioredis 1.x and 2.x. After the author agreed to drop support for aioredis 1.x, I went into a wrong direction that kept the old design of redis cache backend, which managed conn and pool manually. This is required when integrating with aioredis 1.x but not with 2.x
Luckily, I realised it after a while, and refactored the code to be similar with the memecached cache backend. It's much simpler and much more elegant now.
Most of my time is not spent on refactoring the redis cache backend code.
It's taken to make a correct mock of the aioredis.Redis
instance.
FYI, methods like .get()
.set()
on Redis
are not coroutine. In fact,
when we do await redis.get()
we're await its returned value, which is a coroutine.
Autospeccing aioredis.Redis
doesn't exactly reflect the above behavior.
I mocked the .get()
, .set()
methods with AsyncMock
now to pass the tests.
Is there a better way to mock the methods more precisely?
I guess it doesn't matter, cause all we wanna do is to ensure Redis.method
gets called.
Whether Redis.method
communicates with the redis server correctly is tested in aioredis
.
Sorry, I haven't had time to check async support in redis-py
yet.
Done.
The redis >= 4.2.0
async support is based on aioredis
and the APIs are compatible. I switched to from aioredis
to redis
easily. All redis related tests are fixed and passed on my local machine.
There're some breaking changes compared with aioredis 1.x
, I'll detail them as code comment conversions.
Update 1: forgot to update aioredis related part in docs and tox.ini. I'll do it later.
Sorry, I thought the pre-commit hooks was enough to detect the linting err. Maybe some of my commits were made without git hooks installed yet.
Anyway, I fixed the linting err with make lint
. I re-disabled the performance tests. Haven't dig it deeper to fix them yet. Not only redis, there're also some problems with the memcached performance tests as well, during the test running on my local machine.
BTW, I rebased the PR after the "test key refactoring" merged today.
@laggardkernel @argaen Do you guys need someone to take this PR over the finish line? If you want to add me as a maintainer I can finish any remaining tasks necessary and cut a new release. Thx
The PR is completed. Just need another run of the test by the CI, which is not triggered automatically. Part of the performance test is disabled, cause it doesn't fit the connection pool in redis-py
.
@joeblackwaslike Glad about the help you offered. I'm afraid there's nothing we could do. Just the project lacks maintenance. If you need to use aioredis in your own project, you'd better fork the repo, merge this PR and build a package by yourself.
Hi, as mentioned above, I'm not maintaining this project anymore but I'm happy to invite any maintainers who would like to contribute. @laggardkernel @joeblackwaslike let me know and I'll invite you
Hi there,
If someone here looking for alternative with more up to date support:
https://github.com/Krukov/cashews - ready to be used and contribute
- using redis-py
- python 3.7-3.11
I'll refresh myself on where this is at in a couple of days. Tests probably just need to have master merged in order to run, maybe a force-push messed it up or something.
OK, looking good. Just some minor niggles I'd like resolved. Sorry it's taken so long.
Codecov Report
Merging #546 (6c74ed3) into master (760ccab) will decrease coverage by
4.65%
. The diff coverage is86.55%
.
@@ Coverage Diff @@
## master #546 +/- ##
==========================================
- Coverage 98.17% 93.52% -4.66%
==========================================
Files 13 36 +23
Lines 1040 3967 +2927
Branches 140 620 +480
==========================================
+ Hits 1021 3710 +2689
- Misses 15 248 +233
- Partials 4 9 +5
Flag | Coverage Δ | |
---|---|---|
unit | 93.49% <86.55%> (-4.68%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
aiocache/factory.py | 100.00% <ø> (ø) |
|
tests/acceptance/test_factory.py | 100.00% <ø> (ø) |
|
tests/performance/test_concurrency.py | 0.00% <ø> (ø) |
|
tests/performance/test_footprint.py | 0.00% <0.00%> (ø) |
|
aiocache/__init__.py | 83.33% <66.66%> (ø) |
|
aiocache/backends/redis.py | 95.93% <91.52%> (+2.47%) |
:arrow_up: |
tests/acceptance/test_base.py | 100.00% <100.00%> (ø) |
|
tests/performance/conftest.py | 63.63% <100.00%> (ø) |
|
tests/ut/backends/test_redis.py | 100.00% <100.00%> (ø) |
|
tests/ut/test_exceptions.py | 100.00% <0.00%> (ø) |
|
... and 23 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 760ccab...6c74ed3. Read the comment docs.