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.

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

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 this issue Mar 15, 2022 · 3 comments

Comments

@reedstonefood
Copy link

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:

result = @redis.send(command, *args, &block)

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 👍 in the last two comments on #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. sidekiq/sidekiq#4591

@knarewski
Copy link

knarewski commented Mar 22, 2022

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. sidekiq/sidekiq#4591

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

@bf4
Copy link

bf4 commented Aug 31, 2022

related deprecation pr #207

propose close this issue as wontfix

@bf4
Copy link

bf4 commented Aug 31, 2022

xref #171

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

No branches or pull requests

3 participants