#1898·resque

Code & comment for redis initializer method could be clearer

Author: matthewhivelyCreated Mar 14, 2024Updated Mar 14, 2024

https://github.com/resque/resque/blob/2f9d080ce86eb2e3f1f3d47599a21c576124c6f3/lib/resque.rb#L106-L136

I was looking through the initializer for the Resque.redis connection and thought of a few things.

  1. Based on my understanding of the initializer, I believe the comment above this method should be reworded as follows:
# Accepts: 
#   1. A String in any of the following forms
#       a. 'hostname:port'
#       b. 'hostname:port:db'
#       c. 'hostname:port:db/namespace'
#       d. 'hostname:port/namespace'
#       e. 'redis://' or 'rediss://' + 'hostname:port'
#   2. Instances of classes:
#       `Redis`
#       `Redis::Client`
#       `Redis::DistRedis`
#       `Redis::Namespace`
#       `Resque::DataStore`
#   3. A redis connection Hash  {:host => 'localhost', :port => 6379, :db => 0} 
  1. When initializing a regular Redis server, it accepts a string in this form <hostname>:<port>/<db>, no concept of namespace obviously This could lead to confusion since Resque.redis accepts a string in this form <hostname>:<port>:<db>/<namespace> It may be worth a warning in here somewhere of that subtle difference in string definition. Technically this could effect initialization via point 1e above.
  2. The line if server =~ /rediss?\:\/\// could be replaced with if server.start_with?(%r{rediss?://}) for ease of reading (less regex escaping)
  3. It seems odd that the most consistent way to set the namespace is outlined in the README as Resque.redis.namespace = 'foobar'. One would think with the stringified ways of choosing the namespace at init time, there would be a way to do that for initializers other than STRING or Redis::Namespace or Resque::DataStore objects. I don't have a better way to suggest though.