-
-
Notifications
You must be signed in to change notification settings - Fork 937
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
docs(examples/asgilook): update aioredis version to 2.0 #1987
Conversation
Upgrade aioredis library to 2.0 Co-authored-by: Mihai Todor <todormihai@gmail>
Codecov Report
@@ Coverage Diff @@
## master #1987 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 63 63
Lines 6676 6676
Branches 1239 1239
=========================================
Hits 6676 6676 Continue to review full report at Codecov.
|
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 looks really good, thanks!
If possible, I would like to keep an example demonstrating the use of ASGI process_startup
, IIRC we were even linking to that example from FAQ (or were going to).
I understand that aioredis
has changed design so that pool initialization is now a synchronous function. But could we maybe invent a use case for async process_startup
(or alternatively process_shutdown
)?
Maybe we could ping the server in |
Yeah that sounds great @CaselIT ! That makes sense because as I understand I think |
Sounds good! Thanks for the review, we'll plan on adding in that |
Hi again @the-bets , just checking if you are going to have an opportunity to work on that |
Hello again! Thanks for checking in. Mihai and I both got busy with work and life since the last time we talked on this PR. Neither of us have gotten to writing that example yet, and with this being a travel/holiday week for 1/2 of us, we probably won't get to it soon. We do not mind this PR being merged as-is for the time being. If you want to add the example yourselves, that's also fine with us! We are still down to add it, but again likely won't get to it this week. |
Thanks for the update! |
Adds a simple example of using ASGI `process_startup`. (This process is now asynchronous.) As per the discussion above, the example pings the server in `process_startup` and closes the pool in `process_shutdown`. Mihai was able to test it by following the tutorial's test app startup process. Co-authored-by: Mihai Todor <todormihai@gmail.com>
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 LGTM now 💯
(I took the liberty of fixing a couple of trivial/cosmetic issues to get the CI to pass.)
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.
Thanks!
Summary of Changes
Update
aioredis
to version 2.0, the impact of which is found in theasgilook
examples. Apply necessary code and API changes as well as update asgilook tutorial.Both
tools/mintest.sh
andtox --recreate -e asgilook
pass with these updates. Docs (for the tutorial) were also successfully built and changes correctly shown.Related Issues
fixes #1938
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.)If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!
PR template inspired by the attrs project.