amakawa / redic

Lightweight Redis Client
MIT License
120 stars 14 forks source link

Default to REDIS_URL #6

Closed phuongnd08 closed 10 years ago

phuongnd08 commented 10 years ago

The official redis gem redis-rb provide Redis client that try to initialize with ENV["REDIS_URL"] by default. It would be helpful that redic behave like that as well, or at least provide a mechanism so that Redic.new would pickup a user defined value instead of default to localhost:6379

soveran commented 10 years ago

I was one of the guys that added that behavior to redis-rb because it looked very convenient, but now I think otherwise. Having localhost:6379 as the default can be expected, but using different connection details silently may because a lot of problems. Right now, the way to use a REDIS_URL environment variable is like this:

redis = Redic.new(ENV.fetch("REDIS_URL"))

As it is explicit, it is clear what the intention is, and also if the value is not set, an error will be raised as a result of using Hash#fetch (which is to be preferred over Hash#[]).

phuongnd08 commented 10 years ago

I'm not sure how would it cause problems. Can't it just a convention thing?

soveran commented 10 years ago

Imagine an interactive session with IRB. If you want to connect to localhost:6379, as it's now the default, you just do redis = Redic.new. If it is changed so that it reads a value from the environment, then in order to connect to localhost:6379 you would have to be explicit, i.e. redis = Redic.new("redis://localhost:6379"). If you don't want to be explicit, then you would have to check the value of ENV["REDIS_URL"] to make sure it's not set, and only then you would be able to use the direct redis = Redic.new. But in this way, by using that implicit convention would require more care on the side of the developer, because it won't be evident what the state of the environment is. In the end, by being implicit you end up doing more work (checking the status of the environment) or you risk connecting to the wrong database. In my view, not using the environment by default is an improvement on the work we did with redis-rb.

soveran commented 10 years ago

Closing this issue for now, but we can continue the discussion and eventually reopen it.