aiohttp-session icon indicating copy to clipboard operation
aiohttp-session copied to clipboard

Replace aioredis with redis 4.3 or higher

Open achimnol opened this issue 3 years ago • 5 comments

What do these changes do?

It replaces aioredis v2 dependency with redis v4.3 or higher, in favor of official aioredis inclusion in the redis-py project since v4.2.

Are there changes in behavior for the user?

Redis storage users should update their aioredis v2 dependency to redis v4.3.

Related issue number

This is a standalone PR.

Checklist

  • [x] I think the code is well written
  • [x] Unit tests for the changes exist
  • [ ] Documentation reflects the changes
  • [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.

achimnol avatar May 15 '22 10:05 achimnol

Codecov Report

Merging #701 (3329ad2) into master (fdb132a) will decrease coverage by 0.21%. The diff coverage is 100.00%.

:exclamation: Current head 3329ad2 differs from pull request most recent head 740e9d4. Consider uploading reports for the commit 740e9d4 to get more accurate results

@@            Coverage Diff             @@
##           master     #701      +/-   ##
==========================================
- Coverage   98.46%   98.25%   -0.22%     
==========================================
  Files           3        6       +3     
  Lines          65      343     +278     
  Branches        6       51      +45     
==========================================
+ Hits           64      337     +273     
- Misses          0        3       +3     
- Partials        1        3       +2     
Flag Coverage Δ
unit 97.95% <100.00%> (-0.51%) :arrow_down:

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

Impacted Files Coverage Δ
aiohttp_session/redis_storage.py 97.77% <100.00%> (ø)
aiohttp_session/memcached_storage.py 97.77% <0.00%> (ø)
aiohttp_session/__init__.py 98.40% <0.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar May 15 '22 10:05 codecov[bot]

Reported spurious type check failures as python/typeshed#7837.

achimnol avatar May 15 '22 10:05 achimnol

This pull request introduces 1 alert when merging 9c69c443990947a72994a3ef84004d5a7388a1c7 into 657b29bb32305e35fb46b9ceac07f1d03c8d75f8 - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

lgtm-com[bot] avatar May 15 '22 10:05 lgtm-com[bot]

We'll need to fix up the remaining type errors, which might require some more changes to typeshed. I'll take a look at it eventually, if you don't get to it first.

Dreamsorcerer avatar May 17 '22 19:05 Dreamsorcerer

@Dreamsorcerer Could you review this again?

achimnol avatar Jul 06 '22 09:07 achimnol

@achimnol and @Dreamsorcerer,

Is there anything that I can make to help conclude this PR?

amenezes avatar Oct 17 '22 03:10 amenezes

I'll get back to it in the coming weeks, but the tests are failing in current PRs, so we'll need to update and fix those problems as the initial priority (Think it was caused by missing the required check for dependabot, so it managed to merge some broken PRs).

Dreamsorcerer avatar Oct 17 '22 19:10 Dreamsorcerer

OK, someone has sorted out the tests already, had a moment to go through this PR again. Looking good, just a couple of tweaks I'd suggest.

Dreamsorcerer avatar Oct 17 '22 22:10 Dreamsorcerer