Code & comment for redis initializer method could be clearer
Author: matthewhivelyCreated Mar 14, 2024Updated Mar 14, 2024
I was looking through the initializer for the Resque.redis connection and thought of a few things.
- 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} - 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 point1eabove. - The line
if server =~ /rediss?\:\/\//could be replaced withif server.start_with?(%r{rediss?://})for ease of reading (less regex escaping) - 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.
Source: resque/resque