-
-
Notifications
You must be signed in to change notification settings - Fork 948
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
chore: update to latest Redis (aioredis is deprecated) #2204
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2204 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 62 62
Lines 6880 6880
Branches 1099 1099
=========================================
Hits 6880 6880 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kentbull!
Thanks for this [draft] PR! 💯
It looks good for the most of it, just it seems that the CI gate for the tutorial is failing. And one note might need updating (depending on the path that we choose).
You can test the same gate locally as (I've just verified it still works on CPython 3.10):
tox -e asgilook
@@ -747,9 +747,8 @@ implementations for production and testing. | |||
``self.redis_host``. Such a design might prove helpful for apps that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note might need to be updated or even removed altogether if it is no longer needed. How can we hook in fakeredis
for testing then though? (Monkey-patching or mocking might work too.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with the latest update I made the redis_from_url
function is still used. I added a wrapper function to config.py
to change as little as possible. Monkey patching still works.
As of Feb 21, 2023 aioredis-py was archived. See the package repo here: https://github.com/aio-libs-abandoned/aioredis-py
Tests pass, though coverage is not at 100%. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tweaked your code slightly @kentbull, and retested it manually. Now everything seems to pass and work for me.
As of Feb 21, 2023 aioredis-py was archived. See the package repo here:
https://github.com/aio-libs-abandoned/aioredis-py
Summary of Changes
A docs update to correct the ASGI doc to use the latest version of Redis and swap the
aioredis
package for theredis
package sinceaioredis
is now included withinredis
.Pull Request Checklist
This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once; it will save you a few review cycles!
If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.
docs/
.docs/
.versionadded
,versionchanged
, ordeprecated
directives.docs/_newsfragments/
, with the file name format{issue_number}.{fragment_type}.rst
. (Runtowncrier --draft
to ensure it renders correctly.)