redis-rb icon indicating copy to clipboard operation
redis-rb copied to clipboard

Default URL unexpectedly merges with empty options

Open bpo opened this issue 8 years ago • 3 comments

Configuring a Redis client, a user is expected to either use several hash options (:host, :port, etc...) or a :url. I've seen confusion in environments with multiple Redis clients around the code's behavior here:

Expected behavior: If REDIS_URL is defined and I create a new client with Redis.new( host: 'foo', password: 'bar' ), I would expect the other settings to have normal defaults (i.e. port 6379, db 0, etc)

Actual behavior: If REDIS_URL is defined, the resulting Redis client merges the REDIS_URL settings with the options passed to Redis.new.

To demonstrate, in an environment with REDIS_URL = redis://x:[email protected]:86379

Redis.new.client.port == 86379  # expected

redis = Redis.new( host: "dev2", password: "mypass2" )
redis.client.port == 86379  # I would expect this to be 6379

I can submit a patch to fix this minor issue if it would be accepted – wanted to check how that would fit in with some of the other in-flight changes I see in this repo.

bpo avatar Feb 08 '17 23:02 bpo

IMO it was a mistake to even handle the environment variable at all. It is always better to let the user pass this explicitely.

But indeed, if the user passes values, the non-passed ones should use defaults and the env variable should never be touched. In any case (fixing it as mentioned or dropping REDIS_URL support) will be breaking and we can put that in the next major release

badboy avatar Feb 08 '17 23:02 badboy

Sounds good. If you'd like a patch from me for either implementation just ping me when you're ready and let me know which branch to target.

bpo avatar Feb 08 '17 23:02 bpo

I ran into this problem as well.

One solution would be to not use REDIS_URL at all so you don't get unexpected behaviour. But then I have to rewrite my applications and hope that nobody will set it in the future.

Another possibility is to monkey patch the redis gem until the next release is available. I created a file monkey_patch_redis.rb:

require 'redis'

Redis::Client::DEFAULTS[:url] = lambda { 'redis://127.0.0.1:6379' }

What possible problems do you see with this monkey patch approach?

peterfication avatar May 16 '17 13:05 peterfication

This is fixed on master (upcoming 5.0). If either host, port or path is passed $REDIS_URL is ignored.

byroot avatar Aug 17 '22 18:08 byroot