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

fix: Redis#exists(key) returns Integer as the default behaviour #1030

Closed
wants to merge 1 commit into from

Conversation

AkkyOrz
Copy link

@AkkyOrz AkkyOrz commented Oct 6, 2021

Description

redis-rb/lib/redis.rb

Lines 576 to 596 in c97360b

# Determine how many of the keys exists.
#
# @param [String, Array<String>] keys
# @return [Integer]
def exists(*keys)
if !Redis.exists_returns_integer && keys.size == 1
if Redis.exists_returns_integer.nil?
message = "`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. " \
"(#{::Kernel.caller(1, 1).first})\n"
::Kernel.warn(message)
end
exists?(*keys)
else
_exists(*keys)
end
end

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.

After reading the error message, I interpreted it as follows. (The text in parentheses is my own completion)

  • version 4.3 or later, Redis#exists(key) is supposed to return integer.
  • Use Redis#exists?(key) instead (if you want to return a boolean in version 4.3 or later).
  • Set Redis.exists_returns_integer = true if you want to use the "new" behaviour (for users of version 4.3 or earlier)

However, in my environment using redis-rb 4.4, when Redis.exists_returns_integer = nil (default setting), Redis#exists(key) returns boolean with the above error message. I suspect that this is a bug.

When this message was added, the version was prior to 4.3 (4.2.1).
So it would have returned the previous behavior (boolean) when Redis.exists_returns_integer = nil or Redis.exists_returns_integer = false.
However, now (or later) it returns the previous behavior (boolean) only when Redis.exists_returns_integer = false, and not when
I think it should return integer if Redis.exists_returns_integer = nil (default) or Redis.exists_returns_integer = true.

Perhaps Redis.exists_returns_boolean will be more suitable and simpler for opt-in/opt-out applications.

Actually, this is my first pull request, so I may have the wrong way to contribute!
Please let me know if I'm wrong.
Thank you!:blush:

@byroot
Copy link
Collaborator

byroot commented Oct 7, 2021

However, in my environment using redis-rb 4.4, when Redis.exists_returns_integer = nil (default setting), Redis#exists(key) returns boolean with the above error message. I suspect that this is a bug.

Yes, the bug is that I forgot to change the default when I released 4.3 🤦

@byroot
Copy link
Collaborator

byroot commented Oct 7, 2021

Fixed in cf7e628, thank you nonetheless for your PR.

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

Successfully merging this pull request may close these issues.

2 participants