search

resque / redis-namespace

This gem adds a Redis::Namespace class which can be used to namespace Redis keys.
http://redis.io
MIT License
695 stars 192 forks source link

Redis#exists(key) will return an Integer in redis-rb 4.3. exists? returns a boolean, you should use it instead. #195

Open reedstonefood opened 2 years ago

reedstonefood commented 2 years ago

Using version 1.8.1 of this gem, redis 4.2, ruby 2.7.5 and sidekiq 6.4.1. I get this warning a lot in my sidekiq output:

`Redis#exists(key)` will return an Integer in redis-rb 4.3, if you want to keep the old behavior, use `exists?` instead. To opt-in to the new behavior now you can set Redis.exists_returns_integer = true. (/Users/reedstonefood/.gem/ruby/2.7.5/gems/redis-namespace-1.8.1/lib/redis/namespace.rb:476:in `call_with_namespace')

Source code:

https://github.com/resque/redis-namespace/blob/0d853787d73241cefe38fc9a4ba6718c79a9b7b1/lib/redis/namespace.rb#L476

This is a deprecation warning that was apparently fixed in version 1.8. However many people are reporting this error is still shown. This can be seen by the number of :+1: in the last two comments on https://github.com/resque/redis-namespace/issues/172

As there are no further official responses on that closed issue, am raising a new one to raise awareness of this issue & help any future googlers of this message know that they are not alone.

My understanding is that Mike (Sidekiq maintainer) believes this to be a problem with this gem, rather than with sidekiq. https://github.com/mperham/sidekiq/issues/4591

knarewski commented 2 years ago

Hi @reedstonefood ! Let me respond to the best of my understanding:

redis_namespace only acts as a proxy here. The fact that you see this warning means, that some code in your application took your instance of Redis::Namespace and invoked #exists method on it. One of many ways to discover where it's invoked, is to temporarily monkey patch redis-namespace/lib/redis/namespace.rb to print the stacktrace before invoking exists:

pp caller if command.to_s.downcase == 'exists'

Now, both actions you can take have unique consequences:

  1. Set Redis.exists_returns_integer = true - it will opt-in to the new behavior, changing the way Redis#exists works for you; long-term it's a desired solution, but requires reviewing every invocation of Redis#exists and either changing it to Redis#exists? or ensuring that it can correctly handle an integer return value. It also means, that if you ever introduce a gem which is not ready for the new behavior, it will misbehave - that may be a good reason to delay the refactor.
  2. Set Redis.exists_returns_integer = false - it will make Redis stick to the old behavior, which is going to remove the warning for now, but will also completely stop working after reaching certain version of Redis gem. Also it may theoretically misbehave for gems which expect the new behavior (in my opinion that's currently lower risk than the previous point though).

My understanding is that Mike (Sidekiq maintainer) believes this to be a problem with this gem, rather than with sidekiq. https://github.com/mperham/sidekiq/issues/4591

Note that Mike refers to a different warning, which I believe was fixed by officially introducing Redis#exists? to redis_namespace gem

bf4 commented 2 years ago

related deprecation pr https://github.com/resque/redis-namespace/pull/207

propose close this issue as wontfix

bf4 commented 2 years ago

xref https://github.com/resque/redis-namespace/pull/171