Fix: Redis#exists(key) Warning from redis 4.2.0

[MikeRogers0]

This fixes the deprecation warning:

Redis#exists(key) will return an Integer in redis-rb 4.3. exists? returns a boolean, you should use it instead. To opt-in to the new behavior now you can set Redis.exists_returns_integer = true. To disable this message and keep the current (boolean) behaviour of 'exists' you can set Redis.exists_returns_integer = false, but this option will be removed in 5.0. (/usr/local/bundle/gems/sidekiq-cron-1.2.0/lib/sidekiq/cron/job.rb:464:in `block in save')

Approach

I think we're safe to replace instances of Redis#exists(key) with Redis#exists?(key). It has been updated to conditionally change the method being used depending on the Redis version being used.

Try it now

Update your gemfile to use

gem 'sidekiq-cron', github: 'MikeRogers0/sidekiq-cron', branch: 'bug/fix-redis-warning'

Related Tickets

Closes #286

If you liked this PR, please consider buying me a coffee :)

[coveralls]

Coverage increased (+0.02%) to 91.542% when pulling 9957edc on MikeRogers0:bug/fix-redis-warning into 7ca90f9 on ondrejbartas:master.

[iamcz]

@ondrejbartas any idea if this can make it into a patch? Any ideas on the CI? I wonder if this needs to handle multiple versions of Redis and use exists / exists? conditionally on which version is being used. It does not look like there has been any progress on the other MRs for redis-namespace.

[jspanjers]

@ondrejbartas any idea if this can make it into a patch? Any ideas on the CI? I wonder if this needs to handle multiple versions of Redis and use exists / exists? conditionally on which version is being used. It does not look like there has been any progress on the other MRs for redis-namespace.

Would it be enough to just add an explicit dependency on redis >= 4.2.0 to sidekiq-cron?

[lucascaton]

Hi @ondrejbartas! Any news on this?
Is there anything we can do to help? Thanks!

[hirowatari]

@ondrejbartas What can be done to get this completed?
It seems reasonable to me to just bump the redis dependency to at least 4.2 and cut a new gem that requires this.

I'm worried that this gem should be treated as abandoned. Is there anything we can do to help?

[swiknaba]

exists? was only introduced to Redis with version 4.2.0 (June 09, 2020). This gem depends on redis-namespace, which allows redis >= 3.0.4.

Checking out the Travis logs, we see that using Ruby <= 2.4, we get this error:

Redis::CommandError: ERR unknown command `exists?`, with args beginning with: `testy:cron_job:nonexisting`,
    /home/travis/.rvm/gems/ruby-2.4.10/gems/redis-4.1.4/lib/redis/client.rb:126:in `call'

Since this PR now requires Redis >= 4.2 in the gemspec, this should be fine. Strange.

[MikeRogers0]

@swiknaba - Thank you for spotting that duplication :)

My preference right now is drop support for older Ruby/RoR versions if it helps get this update in. Though I've not heard anything from @ondrejbartas :/

@ondrejbartas - Do you have a BuyMeACoffee page? I'd gladly pay a little towards to helping fix this warning :)

[fuegas]

@MikeRogers0 : If you add the following to the PR I think you can resolve the errors in the testsuite:

unless Redis.instance_methods(false).include? :exists?
  class Redis
    alias exists? exists
  end
end

or the following for a short form, though I'd prefer the one above:

class Redis
  alias exists? exists unless instance_methods(false).include? :exists?
end

[swiknaba]

Instead of globally polluting the Redis class, we could also just pull the correct method right on the spot and make clear in the code, that this is due to a change in the Redis gem starting with version 4.2:

REDIS_EXISTS_METHOD = Gem.loaded_specs['redis'].version < Gem::Version.new('4.2') ? :exists : :exists?
conn.send(REDIS_EXISTS_METHOD, job_enqueued_key)

maybe also add an Issue in either case, to remove this hack-around/monkey-patch when dropping support for Redis < 4.2 (after merging this PR).

[MikeRogers0]

I added @swiknaba change in, it passed locally 🤞 travis CI passes.

I'm also going to update this PR to not change the .gemspec file as I don't think we need to change those anymore.

[odarriba]

Thanks for solving this (and for this awesome gem ❤️ )!
Is there any plan to release a new version for this or we should wait for more changes to come?

[miks]

@ondrejbartas is it possible to get gem release? We are getting github auth problems in our build system.

[ac21]

@swiknaba @ondrejbartas - any update on this?

[swiknaba]

@swiknaba @ondrejbartas - any update on this?

I'm not a maintainer and thus, sadly, can't publish a release.

[23tux]

Is there any chance, that a new release gets published in the near future?

[ezekg]

@ondrejbartas let me know if you want to open up sponsors for the project?

Would be happy to sponsor to get new versions cut every once in awhile.

[ryanwilsonperkin]

https://github.com/ondrejbartas/sidekiq-cron/releases/tag/v1.3.0