aioredis-py icon indicating copy to clipboard operation
aioredis-py copied to clipboard

Redis update 2021-10-08

Open Andrew-Chen-Wang opened this issue 3 years ago • 28 comments

What do these changes do?

Updates aioredis to redis at commit andymccurdy/redis-py@9419f1d56beff5b350c87785cc57f2431fe85cbb

This does NOT include "Add retry mechanism with backoff (#1494)"

There may be additional updates to redis-py that merge my PRs there, but they should already be included here.

Are there changes in behavior for the user?

Yes, several new commands

Related issue number

Fixes #1145 #1136 #1130 #1205

Several

Checklist

  • [ ] I think the code is well written
  • [x] Unit tests for the changes exist
  • [ ] Documentation reflects the changes
  • [x] If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • [x] 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.

Andrew-Chen-Wang avatar Oct 08 '21 15:10 Andrew-Chen-Wang

Hey @abrookins ; would be great to get some help to resolve some of these here. Thanks!

Andrew-Chen-Wang avatar Oct 08 '21 17:10 Andrew-Chen-Wang

@Andrew-Chen-Wang You rule, dude! I won't have time to dig into this until early November, but will check in then.

It's probably obvious, but due to other work commitments, I can't be very present here at the moment. I am working with our other Python devs though to try and get a better support plan for this library in place.

abrookins avatar Oct 14 '21 17:10 abrookins

@abrookins sounds great! I've got midterms coming up anyways, so this can wait. Maybe you can at least approve some of the docs PRs? Additionally, might be a good idea to start looking for additional maintainers for this lib (or maybe merge into the redis org like redis-py?)

Andrew-Chen-Wang avatar Oct 14 '21 17:10 Andrew-Chen-Wang

Really excellent work @Andrew-Chen-Wang - I've managed to read through most of this.

I have a couple comments I'll add when I'm at a keyboard, but overall I'm glad to see these changes ported over. It'll really move this implementation forward!

seandstewart avatar Oct 16 '21 14:10 seandstewart

This pull request introduces 5 alerts when merging c3fa5dcd18c7c5e14452a306684edd130a39bef0 into 0212b6a43a4a28d9887f92c55af5e54e1bc95d67 - view on LGTM.com

new alerts:

  • 5 for Unused import

lgtm-com[bot] avatar Nov 04 '21 15:11 lgtm-com[bot]

Codecov Report

Merging #1156 (cb93ef4) into master (56d6b32) will decrease coverage by 0.21%. The diff coverage is 91.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1156      +/-   ##
==========================================
- Coverage   90.66%   90.45%   -0.22%     
==========================================
  Files          21       53      +32     
  Lines        6870    11224    +4354     
  Branches      884     1452     +568     
==========================================
+ Hits         6229    10153    +3924     
- Misses        469      828     +359     
- Partials      172      243      +71     
Flag Coverage Δ
unit 90.42% <91.56%> (-0.16%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aioredis/client.py 82.03% <ø> (-0.44%) :arrow_down:
aioredis/commands/core.py 84.36% <ø> (ø)
aioredis/commands/search/aggregation.py 48.79% <48.79%> (ø)
tests/test_scripting.py 88.18% <50.00%> (-11.82%) :arrow_down:
tests/test_monitor.py 75.47% <54.54%> (-24.53%) :arrow_down:
aioredis/backoff.py 57.44% <57.44%> (ø)
aioredis/commands/search/_util.py 66.66% <66.66%> (ø)
aioredis/utils.py 58.97% <66.66%> (+0.90%) :arrow_up:
aioredis/commands/sentinel.py 67.85% <67.85%> (ø)
tests/conftest.py 86.73% <73.77%> (-5.68%) :arrow_down:
... and 45 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 56d6b32...cb93ef4. Read the comment docs.

codecov[bot] avatar Nov 05 '21 06:11 codecov[bot]

This pull request introduces 5 alerts when merging a76765053ac4c8bc3a905b9c1338d2d317b5ff2b into 034398cec6131a06c1c83059f29729f02d0c9caf - view on LGTM.com

new alerts:

  • 5 for Unused import

lgtm-com[bot] avatar Nov 05 '21 21:11 lgtm-com[bot]

This pull request introduces 1 alert when merging e3a784c1554f1eb7cff5ef0eb029a6d2f8845324 into 034398cec6131a06c1c83059f29729f02d0c9caf - view on LGTM.com

new alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Nov 05 '21 21:11 lgtm-com[bot]

This pull request introduces 1 alert when merging b7dc6bd008472f0f6ce620c0daca039663f55bf3 into 034398cec6131a06c1c83059f29729f02d0c9caf - view on LGTM.com

new alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Nov 05 '21 21:11 lgtm-com[bot]

Only two failing tests remain:

test_bgsave[pool] and test_client_list_client_id[pool]

seandstewart avatar Nov 05 '21 22:11 seandstewart

@seandstewart and @Andrew-Chen-Wang, you guys are the true heroes of aioredis. Apologies that I've not had the time I planned to work on this library after offering to help maintain it. I'm actively trying to find people within Redis who can help in my stead.

Async is important to Redis (the company) and we plan to actually support it with engineering effort, but for some time now our Python folks have had their hands full with regular sync client code in redis-py. I'm trying to get a better community update/plan for async support that we can share publicly.

abrookins avatar Nov 05 '21 22:11 abrookins

This pull request introduces 1 alert when merging 6fd33b0cb0e0209eaa264490523e8c175ef6d8a1 into 034398cec6131a06c1c83059f29729f02d0c9caf - view on LGTM.com

new alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Nov 05 '21 22:11 lgtm-com[bot]

@seandstewart and @Andrew-Chen-Wang, you guys are the true heroes of aioredis. Apologies that I've not had the time I planned to work on this library after offering to help maintain it. I'm actively trying to find people within Redis who can help in my stead.

Async is important to Redis (the company) and we plan to actually support it with engineering effort, but for some time now our Python folks have had their hands full with regular sync client code in redis-py. I'm trying to get a better community update/plan for async support that we can share publicly.

Thanks for the shout-out @abrookins - I completely understand. I'm a huge fan of the work y'all have put into redis-py and I look forward to seeing where y'all take both clients in the future.

seandstewart avatar Nov 05 '21 22:11 seandstewart

This pull request introduces 1 alert when merging 59e0eb4cfa546314f22b5421e2899a32d2779664 into 33b2dbd0a40ac148e6a36ba2fc7ab5d438a9a71d - view on LGTM.com

new alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Nov 20 '21 16:11 lgtm-com[bot]

As of the latest batch of commits, this branch is now up-to-date with redis/redis-py@4e9cc015

seandstewart avatar Nov 21 '21 23:11 seandstewart

This pull request introduces 6 alerts and fixes 4 when merging c8f207440f20dd8a96f7ae6fa76861983440818c into 33b2dbd0a40ac148e6a36ba2fc7ab5d438a9a71d - view on LGTM.com

new alerts:

  • 2 for Conflicting attributes in base classes
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Modification of parameter with default
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 1 for Multiple calls to `__init__` during object initialization
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for First argument to super() is not enclosing class

lgtm-com[bot] avatar Nov 21 '21 23:11 lgtm-com[bot]

This pull request introduces 6 alerts and fixes 4 when merging 18ea63919c0ecc7b536820f35d580080c9d586ec into 33b2dbd0a40ac148e6a36ba2fc7ab5d438a9a71d - view on LGTM.com

new alerts:

  • 2 for Conflicting attributes in base classes
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Modification of parameter with default
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 1 for Multiple calls to `__init__` during object initialization
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for First argument to super() is not enclosing class

lgtm-com[bot] avatar Nov 22 '21 00:11 lgtm-com[bot]

Okay @abrookins @Andrew-Chen-Wang - this PR is up-to-date with [email protected] and we've got a green CI

seandstewart avatar Nov 22 '21 00:11 seandstewart

Woohoo! Tysm @seandstewart . Out of curiosity, what is the reason for the major version bump? And are we hoping to get a release out with just these and previous changes? Hoping we can resolve #1040 first using the contextvar method.

Andrew-Chen-Wang avatar Nov 22 '21 01:11 Andrew-Chen-Wang

Woohoo! Tysm @seandstewart . Out of curiosity, what is the reason for the major version bump? And are we hoping to get a release out with just these and previous changes? Hoping we can resolve #1040 first using the contextvar method.

I think it's reasonable to get a few things out into v2 before we merge this code, but I'm also wary of long-lived PRs.

I'm pretty familiar with contextvars so I'll take a look at that issue and a few others. Perhaps we can push out a 2.1 with some QOL improvements before we merge all this

seandstewart avatar Nov 22 '21 01:11 seandstewart

can we get PR https://github.com/aio-libs/aioredis-py/pull/1129 in this, fixes #1121? @seandstewart @Andrew-Chen-Wang

x0day avatar Nov 22 '21 01:11 x0day

@seandstewart Further PRs will still be using the old layout. I think we should merge this now before further patches and stuff come and this'll be forever delayed. Additionally, I don't think it's necessary to bump to 3.0 yet; I think we should bump version to 2.1.0. The number of changes and additions here isn't breaking anything. We should create a new PR for redis-py 4.0.2+ rather than use this one.

Andrew-Chen-Wang avatar Jan 03 '22 03:01 Andrew-Chen-Wang

This pull request introduces 6 alerts and fixes 4 when merging 817e9b083a4868980594e4e6c9e102b22178dbfa into 56d6b325ee246a3eb0fc8bb6803247c86bb2f494 - view on LGTM.com

new alerts:

  • 2 for Conflicting attributes in base classes
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Modification of parameter with default
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 1 for Multiple calls to `__init__` during object initialization
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for First argument to super() is not enclosing class

lgtm-com[bot] avatar Jan 03 '22 04:01 lgtm-com[bot]

This pull request introduces 6 alerts and fixes 4 when merging 505878abc63ee5a773bd3afda452e3ee9bebb23d into 56d6b325ee246a3eb0fc8bb6803247c86bb2f494 - view on LGTM.com

new alerts:

  • 2 for Conflicting attributes in base classes
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Modification of parameter with default
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 1 for Multiple calls to `__init__` during object initialization
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for First argument to super() is not enclosing class

lgtm-com[bot] avatar Jan 12 '22 01:01 lgtm-com[bot]

This pull request introduces 6 alerts and fixes 4 when merging 61612b1a715bd718c9823e99463f3a26bbdecd46 into 56d6b325ee246a3eb0fc8bb6803247c86bb2f494 - view on LGTM.com

new alerts:

  • 2 for Conflicting attributes in base classes
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Modification of parameter with default
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 1 for Multiple calls to `__init__` during object initialization
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for First argument to super() is not enclosing class

lgtm-com[bot] avatar Jan 12 '22 03:01 lgtm-com[bot]

This pull request introduces 6 alerts and fixes 4 when merging cb93ef4a6fa80f3d32d78aced58d8df196aa58e1 into 56d6b325ee246a3eb0fc8bb6803247c86bb2f494 - view on LGTM.com

new alerts:

  • 2 for Conflicting attributes in base classes
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for Modification of parameter with default
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 1 for Multiple calls to `__init__` during object initialization
  • 1 for Unused import
  • 1 for Missing call to `__init__` during object initialization
  • 1 for First argument to super() is not enclosing class

lgtm-com[bot] avatar Jan 12 '22 03:01 lgtm-com[bot]

We have a ETA when this will hit production?

Urahara avatar Feb 04 '22 00:02 Urahara

This will be included in redis-py's asyncio module. Can't get an ETA. Hopefully end of month.

Andrew-Chen-Wang avatar Feb 10 '22 18:02 Andrew-Chen-Wang