Support `expires_in` for `List`, `Set`, `UniqueList`, `OrderedSet`
Extending the approach taken by https://github.com/rails/kredis/pull/142, now users can set expires_in for List, Set, UniqueList, OrderedSet
Hi @heka1024, thanks for this PR. I was looking for expiration on sets and happy to find your work.
Do you plan to also add expiration to the ActiveModel attributes via the Kredis::Attributes concern? You can grab the code (without extra tests for now) here: https://github.com/bb/kredis/blob/support-ttl/lib/kredis/attributes.rb, just cherry-pick the latest commit.
Also, I'm afraid to get this PR merged, some documentation is needed.
@bb Thanks. I'll chery pick your commit and supplement documentation.
@bb I just cherry-picked your commit and add some test with documentation. Could you review it?
Hey @heka1024, thanks a lot!
The tests you already added in the last commits look good to me. I'm afraid there's no test for ordered set yet.
Docs look also good, I don't think exhaustive examples of all params is needed here, so I'd leave it as is. Keep in mind I'm not a maintainer here, so in the end it's not my decision.
Thanks for adding the missing test @heka1024. LGTM. As said, I'm no maintainer, so I cannot formally approve it.
Maybe @dhh (or another maintainer) can have a look now because his concern about test failures in https://github.com/rails/kredis/pull/133#issuecomment-1872184287 is addressed now.
any reasons why this list doesn't include Hash ?
@balbesina. I just added expiration behavior in hash. I rarely used Redis's hash, I just forgot it 😅.
Hi, @dhh. Could you review this? I fixed https://github.com/rails/kredis/pull/133#issuecomment-1872184287's failing tests.
Although this is a different topic, have you considered adding an expires_at option? I believe it would be beneficial for [product or feature] to also support expires_at. If it hasn't been considered yet, I'm thinking about adding expires_at and would like to follow up on it.
relates https://github.com/rails/rails/pull/41831
@roharon It'll be nice if we can set expires_at! Could you create PR for it?
@heka1024 Cool, I'll work on it.
@heka1024 - Please let me know if you need any help with this PR to get it merged. It will be useful for me too.