-
Notifications
You must be signed in to change notification settings - Fork 35
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
Pass seconds to methods which receive seconds not milliseconds #558
base: 6.5.x
Are you sure you want to change the base?
Conversation
Quality Gate failedFailed conditions |
@@ -223,7 +223,8 @@ public <T> CompletableFuture<T> get(Object key, Argument<T> requiredType, Suppli | |||
Optional<T> deserialized = valueSerializer.deserialize(data, requiredType); | |||
boolean hasValue = deserialized.isPresent(); | |||
if (expireAfterAccess != null && hasValue) { | |||
return redisKeyAsyncCommands.expire(serializedKey, expireAfterAccess).thenApply(ignore -> deserialized.get()); | |||
final long expireAfterAccessInSeconds = expireAfterAccess / 1000; | |||
return redisKeyAsyncCommands.expire(serializedKey, expireAfterAccessInSeconds).thenApply(ignore -> deserialized.get()); |
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.
Instead of converting to milliseconds (which could lose precision), maybe just use pexpire
? (https://redis.io/docs/latest/commands/pexpire/)
Update - or I guess you mention something about pexpire
in the PR description, but it isn't clear to me why not to use it here instead.
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.
indeed , using pexpire would solve the issue rather than converting to milliseconds, I've added tests that hit the methods .
Quality Gate passedIssues Measures |
Close: #474
RedisAsynCommand::expire
methods takes seconds but we were passing milliseconds.There are other methods in the Redis API, for example the
RedisAsynCommand::pexpire
which takes milliseconds. Our tests, were hitting that method. That it is the reason why they were passing.This PR converts from milliseconds to seconds the methods that require seconds and it adds tests for those code paths.