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.

Sign up for GitHub

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

Jump to bottom

[Feature Request] Support GETEX command that available since Redis 6.2.0 #1740

Closed
PMExtra opened this issue May 30, 2021 · 0 comments · Fixed by #1743
Closed

[Feature Request] Support GETEX command that available since Redis 6.2.0 #1740

PMExtra opened this issue May 30, 2021 · 0 comments · Fixed by #1743
Labels
⚙️ area:commands 🙌 up-for-grabs

Comments

@PMExtra
Copy link

PMExtra commented May 30, 2021

https://redis.io/commands/getex

It's a useful feature!
@benbryant0 benbryant0 mentioned this issue Jun 2, 2021
Merged
NickCraver added a commit that referenced this issue Mar 28, 2022
@benbryant0 @NickCraver
Implement GETEX command (#1743)
b159173 
Resolves #1740

Should add support for all current options of [GETEX](https://redis.io/commands/getex).

Questionable (non-)decisions (opinions wanted!):
* I called the new methods `StringGet[Async]` as the behaviour is most similar to plain `StringGet` and I think the added expiry functionality is communicated by the parameters.
* The relative expiry overload takes `TimeSpan?` while the absolute overload takes non-nullable `DateTime`. I don't like it, but I did this as I realised I'd have to cast to escape ambiguity: `(TimeSpan?)null` 🤮. But maybe consistency is more important?
* I looked in the `tests` folder and cried a little; I have no idea what I'm supposed to update there. I just bumped the Redis version in `Dockerfile` since Docker is easy and you seem to use it in your GitHub actions.
* I placed the tests in `Strings`, even though they involve expiry and the tests themselves go against the pattern in that file a bit. Felt better than bloating something catch-all like `BasicOps`.

Co-authored-by: Nick Craver <nrcraver@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
⚙️ area:commands 🙌 up-for-grabs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement GETEX command
2 participants
@NickCraver @PMExtra