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

Deprecate == and != from Redis::Future #876

Merged
merged 2 commits into from Sep 19, 2019

Conversation

GustavoCaso
Copy link
Contributor

@GustavoCaso GustavoCaso commented Sep 19, 2019

For more information please check #875

Most developers when using == after calling a redis command they expect to compare the result returned from Redis.

Inside a pipelined, we are operating with Redis::Future instances that are BasicObject

BasicObject#== is doing identity comparison instead of value comparison which is less intuitive for developers.

Example:

result = nil

redis.pipelined do 
  result = redis.set('foo', 'bar') == "OK"
end

puts result 
# => false

By removing the ability to use == with a redis future developers would get NoMethodError and this helps to have more consistent errors when operating with Redis::Future inside a pipelined block

This helps to have a more consistent errors when operating with
Redis::Future inside a pipelined block
@casperisfine
Copy link

A couple things:

  • I'm generally favorable to this, however redis being very widely used, I think it needs a deprecation period
  • There is also the != method
  • People can get tricked if they use Yoda comparisons "OK" == redis.set('foo', 'bar'), but I guess that's fine.

@GustavoCaso GustavoCaso changed the title Remove method == from Redis::Future Deprecate == and != from Redis::Future Sep 19, 2019
@GustavoCaso
Copy link
Contributor Author

Example of trying to call the == or != method

bin/console
irb(main):001:0> redis = Redis.new
=> #<Redis client v4.1.3 for redis://127.0.0.1:6379/0>
irb(main):002:0> redis.pipelined do
irb(main):003:1* redis.set("foo", "bar") == 'OK"
irb(main):004:1' end
irb(main):005:1' ^C
irb(main):005:0> redis.pipelined do
irb(main):006:1* redis.set("foo", "bar") == 'OK'
irb(main):007:1> end

The method == and != is deprecated for Redis::Future and will be removed in 4.2.0 - You probably meant to call .value == or .value != (in /Users/gustavocaso/src/github.com/redis-rb/lib/redis/pipeline.rb:145:in `==')
=> ["OK"]
irb(main):008:0> redis.pipelined do
irb(main):009:1* redis.set("foo", "bar") != 'OK'
irb(main):010:1> end

The method == and != is deprecated for Redis::Future and will be removed in 4.2.0 - You probably meant to call .value == or .value != (in /Users/gustavocaso/src/github.com/redis-rb/lib/redis/pipeline.rb:145:in `==')
=> ["OK"]
irb(main):011:0>

@GustavoCaso
Copy link
Contributor Author

I didn't have to deprecate != explicitly since != uses == underneath and we will see two deprecated messages.

@byroot byroot merged commit 6b894f8 into redis:master Sep 19, 2019
@GustavoCaso GustavoCaso deleted the undef-==-from-redis-future branch September 19, 2019 14:51
jessedoyle pushed a commit to amaabca/ama_layout that referenced this pull request Mar 5, 2021
* Fix a few deprecation warnings that were introduced in version
  4.2.0 of the redis gem [1].
* As the data store API is internal, no longer check the result of
  a single step of a multi-transaction commit in redis. Remove
  the corresponding specs as well.

[1]: redis/redis-rb#876
SEE: https://dev.azure.com/AMA-Ent/AMA-Ent/_workitems/edit/19603
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.

None yet

3 participants