Skip to content
This repository has been archived by the owner on Jan 1, 2024. It is now read-only.

[Reverted] Use Redis#exists? and fallback to #exists which could be either boolean or integer in recent redis gem versions #382

Merged
merged 1 commit into from Apr 22, 2022

Conversation

bsedin
Copy link
Contributor

@bsedin bsedin commented Apr 14, 2022

Hello!

First of all thanks for this amazing library!

I'm trying to upgrade redis from 4.4 to 4.6 in my project. It's rather painless but I noticed Vanity wasn't working properly as it assumes exists to return true or false which in recent redis-rb versions was changed and now it returns number of matching keys unless Redis.exists_returns_integer is set to false. But that option is deprecated and will be removed in redis-rb 5.0.

So for boolean checks it is recommended to use exists? method and right now it's a bit messy with all that possibilities out there. So I tried not to break anything by checking for exists? method which we now for sure gives us boolean result. And if that method is absent, we're probing exists and if it's numeric we are calculating > 0 if not we're assuming it's boolean.

So what do you think about that?

which could be either boolean or integer
@bensheldon
Copy link
Collaborator

@bsedin thanks for flagging this and proposing a fix. I think the redis-namespace gem needs to be updated too to get the tests to pass: resque/redis-namespace#171

I'm thinking we should add a Github Action matrix option to test both versions of Redis (we likely don't need to test every combo, just one with the new and old Redis).

@bensheldon bensheldon merged commit bb6ae55 into assaf:master Apr 22, 2022
@bensheldon
Copy link
Collaborator

Whoops, didn't realize pushing that would merge it into master 🤦

@bsedin
Copy link
Contributor Author

bsedin commented Apr 22, 2022

@bensheldon Hey Ben and many thanks for fast response!
Sorry my code caused master branch breaking :( Could not find time this week to finish this.

I owe you this one and I'll try to redeem myself by contributing something in future!

@bensheldon bensheldon changed the title Use Redis#exists? and fallback to #exists which could be either boolean or integer in recent redis gem versions [Reverted] Use Redis#exists? and fallback to #exists which could be either boolean or integer in recent redis gem versions Apr 22, 2022
@bensheldon
Copy link
Collaborator

@bsedin no worries. it was my bad. I tried to push up a fix to your branch and it ended up going to the master branch 🤦

I made a new PR (#385) and it's looking very good. I'll have a release out this morning. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants