Skip to content

Add API to remove expiration for RedisKeyWritable#276

Merged
MeirShpilraien merged 1 commit into
RedisLabsModules:masterfrom
QuChen88:expire
Feb 1, 2024
Merged

Add API to remove expiration for RedisKeyWritable#276
MeirShpilraien merged 1 commit into
RedisLabsModules:masterfrom
QuChen88:expire

Conversation

@QuChen88
Copy link
Copy Markdown
Contributor

@QuChen88 QuChen88 commented Feb 4, 2023

Currently RedisKeyWritable has no easy way to remove expiration/TTL from a key that already exists in Redis because set_expire() takes Duration as input even though the underlying raw::set_expire() method supports this functionality.

This PR adds a new method remove_expire() to RedisKeyWritable.

@QuChen88 QuChen88 changed the title Add new method to remove expiration for RedisKeyWritable Add API to remove expiration for RedisKeyWritable Feb 4, 2023
@QuChen88
Copy link
Copy Markdown
Contributor Author

QuChen88 commented Apr 6, 2023

Ping, any updates on this?

iddm
iddm previously approved these changes Apr 13, 2023
Comment thread src/key.rs
iddm
iddm previously approved these changes May 2, 2023
Copy link
Copy Markdown
Contributor

@iddm iddm left a comment

Choose a reason for hiding this comment

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

Thank you!

@QuChen88
Copy link
Copy Markdown
Contributor Author

QuChen88 commented Jan 11, 2024

@MeirShpilraien Can you please take a look? I just rebased the change.

MeirShpilraien
MeirShpilraien previously approved these changes Jan 15, 2024
@MeirShpilraien
Copy link
Copy Markdown

MeirShpilraien commented Jan 15, 2024

@QuChen88 maybe we can add a test for the new functionality?

@QuChen88
Copy link
Copy Markdown
Contributor Author

I can't seem to find an existing test for the set_expire() method from before.

What is the convention for adding such a test? Can you point me to an example?

@MeirShpilraien
Copy link
Copy Markdown

@QuChen88 what we usually do is adding a simple example module under the example directory, the module can expose a simple command that uses the new functionality (like a command that remove expiration from a key). Then we add a test that loads the module and check the functionality here: https://github.com/RedisLabsModules/redismodule-rs/blob/master/tests/integration.rs

@MeirShpilraien
Copy link
Copy Markdown

freebds flow seems broken (stuck). Merging without it (tests pass on the other flows).

@MeirShpilraien MeirShpilraien merged commit bf2b3a6 into RedisLabsModules:master Feb 1, 2024
@MeirShpilraien
Copy link
Copy Markdown

Thanks @QuChen88

@QuChen88 QuChen88 deleted the expire branch February 1, 2024 18:20
@github-actions github-actions Bot mentioned this pull request May 4, 2026
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