Python: adds PTTL command #1036
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shohamazon
commented
Feb 25, 2024
| @@ -926,14 +926,15 @@ async def test_unlink(self, redis_client: TRedisClient): | |||
|
|
|||
| @pytest.mark.parametrize("cluster_mode", [True, False]) | |||
| @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) | |||
| async def test_expire_pexpire_ttl_with_positive_timeout( | |||
| async def test_expire_pexpire_ttl_pttl_with_positive_timeout( | |||
There was a problem hiding this comment.
please tell me if you prefer:
@pytest.mark.parametrize("cluster_mode", [True, False])
@pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3])
async def test_pttl(self, redis_client: TRedisClient):
key = get_random_string(10)
assert await redis_client.pttl(key) == -2
current_time = int(time.time())
assert await redis_client.set(key, "value") == OK
assert await redis_client.pttl(key) == -1
assert await redis_client.expire(key, 10) == True
assert 0 < await redis_client.pttl(key) <= 10000
assert await redis_client.expireat(key, current_time + 20) == True
assert 0 < await redis_client.pttl(key) <= 20000
assert await redis_client.pexpireat(key, current_time * 1000 + 30000) == True
assert 0 < await redis_client.pttl(key) <= 30000
# set command clears the timeout.
assert await redis_client.set(key, "value") == OK
assert await redis_client.pttl(key) == -1
There was a problem hiding this comment.
Yes, separating the command to a different test will help debugging if needed later on. The last check though seems duplicated (set and then asserting again that its PTTL is -1)
shohamazon
force-pushed
the
python/pttl
branch
from
a80f28d
to
3857d09
Compare
| @@ -1005,7 +1005,7 @@ async def expire( | |||
|
|
|||
| Examples: | |||
| >>> await client.expire("my_key", 60) | |||
| 1 # Indicates that a timeout of 60 seconds has been set for "my_key." | |||
| True # Indicates that a timeout of 60 seconds has been set for "my_key." | |||
There was a problem hiding this comment.
May be move doc fixes to another PR?
There was a problem hiding this comment.
+1 for next time, for now just make sure to have these changes in a separate commit
| else: | ||
| assert ( | ||
| await redis_client.expireat( | ||
| key, current_time + 50, ExpireOptions.NewExpiryGreaterThanCurrent | ||
| ) | ||
| == 1 | ||
| == True |
There was a problem hiding this comment.
You can do assert await ... without == True
barshaul
approved these changes
Mar 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.