Skip to content
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

Support [NX|XX|GT|LT] options in EXPIREAT and PEXPIREAT #2874

Merged
merged 6 commits into from
Feb 7, 2022

Conversation

Avital-Fine
Copy link
Contributor

No description provided.

@Avital-Fine Avital-Fine requested a review from sazzad16 February 6, 2022 14:34
Copy link
Collaborator

@sazzad16 sazzad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep all At method together. I.e.

  • expireAt(String key, long unixTime);
  • expireAt(String key, long unixTime, ExpiryOption expiryOption);
  • pexpireAt(String key, long millisecondsTimestamp);
  • pexpireAt(String key, long millisecondsTimestamp, ExpiryOption expiryOption);

@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2022

Codecov Report

Merging #2874 (66c3007) into master (e3fafc6) will decrease coverage by 0.15%.
The diff coverage is 51.85%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2874      +/-   ##
============================================
- Coverage     56.71%   56.56%   -0.16%     
- Complexity     3148     3152       +4     
============================================
  Files           189      189              
  Lines         11699    11727      +28     
  Branches        676      676              
============================================
- Hits           6635     6633       -2     
- Misses         4837     4865      +28     
- Partials        227      229       +2     
Impacted Files Coverage Δ
...ava/redis/clients/jedis/MultiNodePipelineBase.java 25.20% <0.00%> (-0.14%) ⬇️
src/main/java/redis/clients/jedis/Pipeline.java 8.84% <0.00%> (-0.05%) ⬇️
...main/java/redis/clients/jedis/TransactionBase.java 12.92% <0.00%> (-0.07%) ⬇️
.../main/java/redis/clients/jedis/CommandObjects.java 69.30% <66.66%> (-0.08%) ⬇️
...rc/main/java/redis/clients/jedis/UnifiedJedis.java 60.59% <66.66%> (-0.06%) ⬇️
src/main/java/redis/clients/jedis/Jedis.java 84.79% <83.33%> (-0.15%) ⬇️
...ava/redis/clients/jedis/params/FailoverParams.java 76.19% <0.00%> (-23.81%) ⬇️
...in/java/redis/clients/jedis/ConnectionFactory.java 63.26% <0.00%> (-4.09%) ⬇️
src/main/java/redis/clients/jedis/Connection.java 83.83% <0.00%> (-1.52%) ⬇️

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 e3fafc6...66c3007. Read the comment docs.

Avital-Fine and others added 3 commits February 6, 2022 16:20
expire(String key, long seconds);
expire(String key, long seconds, ExpiryOption expiryOption);
pexpire(String key, long milliseconds);
pexpire(String key, long milliseconds, ExpiryOption expiryOption);
expireTime(String key);
pexpireTime(String key);
expireAt(String key, long unixTime);
expireAt(String key, long unixTime, ExpiryOption expiryOption);
pexpireAt(String key, long millisecondsTimestamp);
pexpireAt(String key, long millisecondsTimestamp, ExpiryOption expiryOption);
@Avital-Fine
Copy link
Contributor Author

Please keep all At method together. I.e.

  • expireAt(String key, long unixTime);
  • expireAt(String key, long unixTime, ExpiryOption expiryOption);
  • pexpireAt(String key, long millisecondsTimestamp);
  • pexpireAt(String key, long millisecondsTimestamp, ExpiryOption expiryOption);

I changed it so now the order of all the expire commands is:

  • expire(String key, long seconds);
  • expire(String key, long seconds, ExpiryOption expiryOption);
  • pexpire(String key, long milliseconds);
  • pexpire(String key, long milliseconds, ExpiryOption expiryOption);
  • expireTime(String key);
  • pexpireTime(String key);
  • expireAt(String key, long unixTime);
  • expireAt(String key, long unixTime, ExpiryOption expiryOption);
  • pexpireAt(String key, long millisecondsTimestamp);
  • pexpireAt(String key, long millisecondsTimestamp, ExpiryOption expiryOption);

@Avital-Fine Avital-Fine requested a review from sazzad16 February 6, 2022 15:30
Copy link
Collaborator

@sazzad16 sazzad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it so now the order of all the expire commands is:
...
...

@Avital-Fine You've missed some orderings in Jedis.java and Pipeline.java

@Avital-Fine Avital-Fine requested a review from sazzad16 February 6, 2022 16:31
@sazzad16 sazzad16 added this to the 4.2.0 milestone Feb 7, 2022
@sazzad16 sazzad16 merged commit 58245f3 into redis:master Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants