assaf / vanity

Experiment Driven Development for Ruby
http://vanity.labnotes.org
MIT License
1.55k stars 269 forks source link

Redis gem version 4+ ==> Error when Vanity tries to call redis.client.disconnect! #350

Closed mfeldman59 closed 5 years ago

mfeldman59 commented 5 years ago

In the RedisAdapter class, the disconnect! method uses the call redis.client.disconnect! which used to work with Redis gem versions < 3.

With Redis gem version > 4 , they changed the code and this call will fail with the error: Error while disconnecting from redis: ERR Syntax error, try CLIENT (LIST | KILL | GETNAME | SETNAME | PAUSE | REPLY)

I ran in to this as our Rails app runs in a docker container using passenger-phusion

SOLUTION: In the disconnect! method, replace redis.client.disconnect! with redis.disconnect!

mfeldman59 commented 5 years ago

And for those that run into this issue, I simply created a vanity initializer that "patches" the affected method until Vanity gem can get released with a proper fix:

config/initializers/vanity.rb:

require 'vanity/adapters/abstract_adapter'
require 'vanity/adapters/redis_adapter'

######################################
#  Patching the Vanity GEM to play nicely with the Redis gem > 4
#
#  The problem:
#   Vanity's RedisAdapter class has a disconnect! method that is meant to close
#   the current connection to the redis server. In the Redis Gem upgrade to version 4+
#   the gem changed how you get access to the redis client object.
#   Previous to version 4, the Redis gem had a getter/setter to get the client.
#   Post version 4, you do not need to get the client. BUT, what they did was create a
#   method called 'client()'. This breaks any code that used a calling signature such as
#   `Redis.client.disconnect!`
#
#  The solution:
#   Since the Vanity gem tries to use redis.client.disconnect!, we can override the offending code
#   and use redis.disconnect! instead.
#
######################################
# TODO: Check back every so often to see if Vanity has updated the Gen.

module Vanity
  module Adapters
    class RedisAdapter < AbstractAdapter
      def disconnect!
        if redis
          begin
            # redis.client.disconnect!
            redis.disconnect!
          rescue Exception => e
            Vanity.logger.warn("Error while disconnecting from redis: #{e.message}")
          end
        end
        @redis = nil
      end
    end
  end
end
phillbaker commented 5 years ago

@mfeldman59 would you like to make a PR with this change?

mfeldman59 commented 5 years ago

Sure thing - i'll have to setup a dev env for the project.

phillbaker commented 5 years ago

Specifically in v4.0, #client changed meaning: https://github.com/redis/redis-rb/blob/master/CHANGELOG.md#40.

disconnect! is backwards compatible.