Skip to content
This repository has been archived by the owner on Feb 21, 2023. It is now read-only.

Implementation of redis locks (for single redis masters) #573

Closed
wants to merge 12 commits into from
Closed

Implementation of redis locks (for single redis masters) #573

wants to merge 12 commits into from

Conversation

mattrasband
Copy link

@mattrasband mattrasband commented Apr 11, 2019

What do these changes do?

Provide an implementation of locking utilizing redis

Are there changes in behavior for the user?

Users now have a .lock method on the GenericCommandsMixin, but it's optional path.

Related issue number

#43

Checklist

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

@codecov
Copy link

codecov bot commented Apr 11, 2019

Codecov Report

Merging #573 (5924033) into master (e8c33e3) will decrease coverage by 0.22%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #573      +/-   ##
==========================================
- Coverage   96.86%   96.64%   -0.23%     
==========================================
  Files          57       57              
  Lines        8431     8051     -380     
  Branches      567      571       +4     
==========================================
- Hits         8167     7781     -386     
- Misses        188      191       +3     
- Partials       76       79       +3     
Impacted Files Coverage Δ
tests/generic_commands_test.py 99.63% <83.33%> (-0.37%) ⬇️
aioredis/locks.py 93.93% <90.90%> (-6.07%) ⬇️
aioredis/commands/generic.py 100.00% <100.00%> (ø)
aioredis/errors.py 100.00% <100.00%> (ø)
tests/conftest.py 88.29% <0.00%> (-0.68%) ⬇️
tests/connection_commands_test.py 95.45% <0.00%> (-0.55%) ⬇️
tests/sentinel_failover_test.py 87.31% <0.00%> (-0.55%) ⬇️
aioredis/commands/__init__.py 95.18% <0.00%> (-0.33%) ⬇️
tests/pyreader_test.py 96.87% <0.00%> (-0.27%) ⬇️
aioredis/sentinel/pool.py 76.80% <0.00%> (-0.27%) ⬇️
... and 29 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 e8c33e3...5924033. Read the comment docs.

@iMerica
Copy link

iMerica commented Apr 12, 2019

LGTM 👍

@popravich popravich self-requested a review April 15, 2019 13:33
@popravich popravich self-assigned this Apr 15, 2019
@mattrasband
Copy link
Author

Updated to resolve conflicts/changes from master

@mattrasband mattrasband mentioned this pull request Jun 9, 2019
@seandstewart
Copy link
Collaborator

@mrasband -

Thanks for the contribution! Would you be willing to rebase your changes off of master and resolve the merge conflicts?

You may be asked to sign a CLA after which I think we can move forward with merging it in!

@CLAassistant
Copy link

CLAassistant commented Nov 16, 2020

CLA assistant check
All committers have signed the CLA.

@mattrasband
Copy link
Author

@seandstewart Yep, I will get to that sometime today or tomorrow!

@mattrasband
Copy link
Author

This out of date enough I will close and reopen.

@mattrasband mattrasband deleted the 43-locks-in-redis branch December 27, 2020 03:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants