amakawa / redic

Lightweight Redis Client
MIT License
120 stars 14 forks source link

Allow URL from ENV['REDIS_URL'] #1

Closed Asmod4n closed 10 years ago

frodsan commented 10 years ago

:-1: Redic.new(ENV.fetch('REDIS_URL'))?

soveran commented 10 years ago

We considered reading REDIS_URL directly. The problem we had with that approach is that it can be a source of bugs. The two approaches that we could use are the following:

ENV["REDIS_URL"] || "redis://127.0.0.1:6379"

or

ENV.fetch("REDIS_URL", "redis://127.0.0.1:6379")

Both behave the same, and the problem is that if REDIS_URL is not defined, you end up using the default host without noticing. Instead, if you don't pass any parameters to Redic.new, it is explicit that you want to use the default host. At the same time, if you want to use the value of REDIS_URL, it is better (in terms of writing maintainable code) to be explicit and use fetch:

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

Because that way, you will get an exception if ENV["REDIS_URL"] is nil.

frodsan commented 10 years ago

:+1:

cyx commented 10 years ago

@Asmod4n thoughts? I personally had some bugs with this default, and most of the time you end up doing lazy programming -- ending up in a scenario where you can't reason about which redis gets used at whatever point.

Asmod4n commented 10 years ago

Fine with closing it, any chance though this might work with Jruby or on Windows at all in the future?

soveran commented 10 years ago

@Asmod4n What's the error you get on those platforms?

Asmod4n commented 10 years ago

Hiredis is a C extension which only compiles on Unix, Mingw/VisualStudio lack some header files and of course Unixsocket support is missing there.

soveran commented 10 years ago

Did you try it in JRuby? I ask because the gemspec skips the compilation if it's in java, and uses a pure ruby version instead.

Asmod4n commented 10 years ago

Oh, neet, will try again later. :+1:

soveran commented 10 years ago

Great, let me know :-)