- Notifications
You must be signed in to change notification settings - Fork 2.6k
add async Retry __eq__ and __hash__ & fix ExponentialWithJitterBackoff __eq__ #3668
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
Conversation
__eq____eq__ and __hash__ & fix ExponentialWithJitterBackoff __eq__ b32dc30 to 0278734 Compare 3d47594 to d51896e Compare This change fixes a typo in `ExponentialWithJitterBackoff`'s `__eq__` method.
9f675cc to 1e2452b Compare …errors more obvious
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.
Pull Request Overview
This PR fixes a typo in the ExponentialWithJitterBackoff.__eq__ method and refactors the retry system to support __eq__ and __hash__ methods for async Retry objects. The changes introduce a shared base class to eliminate code duplication between sync and async retry implementations.
- Extracts common retry functionality into an
AbstractRetrybase class with generic type support - Fixes incorrect class name check in
ExponentialWithJitterBackoff.__eq__method - Updates tests to verify both sync and async
Retryobjects support equality and hashing operations
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| redis/retry.py | Refactors to introduce AbstractRetry base class and updates Retry to inherit from it |
| redis/asyncio/retry.py | Simplifies async Retry by inheriting from AbstractRetry and implementing __eq__ method |
| redis/backoff.py | Fixes typo in ExponentialWithJitterBackoff.__eq__ method class name check |
| tests/test_retry.py | Parameterizes existing tests to cover both sync and async Retry classes |
redis/retry.py Outdated
| __init__.__doc__ = AbstractRetry.__init__.__doc__ | ||
| |
Copilot AI Jul 22, 2025
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.
[nitpick] Manually copying docstrings can lead to maintenance issues. Consider using proper inheritance or decorators to share documentation, or simply let the docstring inherit naturally from the parent class.
| __init__.__doc__ = AbstractRetry.__init__.__doc__ |
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 removed both of these, but I'm not sure this is actually correct. This is supposed to be the case for class docstrings, but this is a class method and testing locally (I wasn't sure of this before the review and tested myself) it did not inherit the docstring (at least when using the help method).
…erBackoff ``__eq__`` (#3668) * fix ExponentialWithJitterBackoff ``__eq__`` This change fixes a typo in `ExponentialWithJitterBackoff`'s `__eq__` method. * create abstract retry class to share methods between retry implementations * update retry classes: remove slots & move `__eq__` to concrete classes * implement __init__ methods in retry subclasses to make the supported errors more obvious
Description of change
This change fixes a typo in
ExponentialWithJitterBackoff's__eq__method from #3628, and allows asyncRetryobjects to also support__eq__and__hash__.Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.