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

Feat: GETEX and GETDEL Added #582

Merged
merged 2 commits into from Jun 6, 2022
Merged

Feat: GETEX and GETDEL Added #582

merged 2 commits into from Jun 6, 2022

Conversation

arpandaze
Copy link
Contributor

This PR adds the following commands:

  1. GETEX (Available since 6.2.0)
  2. GETDEL (Available since 6.2.0)

Closes #485

@arpandaze
Copy link
Contributor Author

Hi @djc ! Can you please approve this?

Copy link
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

I think this should come with some tests.

src/commands.rs Outdated
@@ -397,6 +397,16 @@ implement_commands! {
cmd("PTTL").arg(key)
}

/// Get the value of a key and set expiration
fn get_ex<K: ToRedisArgs>(key: K, seconds: usize) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For get_ex, I think it would be good to support EX/PX/EXAT/PXAT and PERSIST options as well. For the former, it probably makes sense to define an enum, or maybe a custom Duration type with some simple constructors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have now added support for those options and also created tests for get_ex and get_del. CI shows that build failed on 1.51.0 but I think that is due to unrelated issue. Can you please review the changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the MSRV issue is #624.

src/lib.rs Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/types.rs Show resolved Hide resolved
src/commands.rs Outdated Show resolved Hide resolved
@arpandaze
Copy link
Contributor Author

I have made the requested changes.

@djc djc merged commit e44d5ff into redis-rs:main Jun 6, 2022
@djc
Copy link
Contributor

djc commented Jun 6, 2022

Thanks!

nonirosenfeldredis pushed a commit to nonirosenfeldredis/redis-rs that referenced this pull request Feb 13, 2023
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.

GETDEL command support
2 participants