Fix:Expire method fails when using DEFAULT_TIMEOUT #726
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #726 +/- ##
========================================
- Coverage 61.6% 61.5% -0.1%
========================================
Files 43 43
Lines 2773 2786 +13
Branches 161 161
========================================
+ Hits 1708 1713 +5
- Misses 1052 1060 +8
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@WisdomPill Whenever it's convenient for you, could you please take a moment to review and approve the workflow? Your approval would be greatly appreciated. Thank you! |
django_redis/client/default.py
Outdated
| # timeout could be of type int|timedelta | ||
| # if timeout is DEFAULT_TIMEOUT or None then | ||
| # use self._backend.default_timeout(django default cache timeout) |
changelog.d/724.bugfix
Outdated
| @@ -0,0 +1,3 @@ | |||
| Fix for Issue 724(expire method fails when using DEFAULT_TIMEOUT) | |||
|
|
|||
| https://github.com/jazzband/django-redis/issues/724 | |||
There was a problem hiding this comment.
no need to point out the issue just write an explanation
django_redis/client/default.py
Outdated
| # for some strange reason mypy complains, | ||
| # saying that timeout type is float | timedelta |
| @@ -606,6 +607,10 @@ def test_expire(self, cache: RedisCache): | |||
| assert pytest.approx(ttl) == 20 | |||
| assert cache.expire("not-existent-key", 20) is False | |||
|
|
|||
| def test_expire_with_default_timeout(self, cache: RedisCache): | |||
| cache.set("foo", "bar", timeout=None) | |||
| assert cache.expire("foo", DEFAULT_TIMEOUT) is True | |||
There was a problem hiding this comment.
can you try to expire a non existing key as well?
WisdomPill
changed the title
…scases & changelog updated
|
@WisdomPill Your valuable input has been carefully integrated. Whenever you have a moment, could you kindly review and approve the workflow? Your cooperation is sincerely appreciated. Thank you! |
django_redis/client/default.py
Outdated
| @@ -315,6 +318,9 @@ def pexpire( | |||
| version: Optional[int] = None, | |||
| client: Optional[Redis] = None, | |||
| ) -> bool: | |||
| if (timeout is DEFAULT_TIMEOUT) or (timeout is None): | |||
There was a problem hiding this comment.
I would let people set None as timeout and get the exception from the client so that they notice they made a mistake instead of getting a seemingly random expiry time
There was a problem hiding this comment.
Thanks for your feedback @WisdomPill!
Changes are done. Please approve the workflow.
closes #724