sorentwo / readthis

:newspaper: Pooled active support compliant caching with redis
MIT License
504 stars 40 forks source link

Consider not defaulting to hiredis since it isn't included as a dependency #9

Closed monfresh closed 9 years ago

monfresh commented 9 years ago

Hi. This gem defaults to hiredis, but since hiredis is no longer included as a dependency, if I don't have hiredis in my Gemfile, then the connection will fail. In my opinion, it doesn't make sense to default to hiredis when it isn't included as a dependency by default. In order for readthis to work out of the box, it is required for a client app to add hiredis to their Gemfile. Therefore, why not include it as a dependency by default?

If you don't want to have hiredis as a dependency for readthis, then I think the default should be the ruby driver, and if people want to use hiredis, they can add the gem to their Gemfile and specify the driver during initialization.

Thoughts?

sorentwo commented 9 years ago

@monfresh: I think that's a very sensible approach. I don't want hiredis as a dependency for possible jruby compatibility. Hiredis is a transparent replacement that I'd encourage anybody to use, but I don't want to mandate it.

monfresh commented 9 years ago

Wonderful! Thanks for the fast update. By the way, this gem rocks! I've been testing it for use with rack-cache, which required me to fork the redis-rack-cache gem and modify it, but once I got it working, I compared it to dalli + memcached and redis-store, and it blew all of them out of the water. readthis was about 4x faster than redis-store and 8x faster than Dalli+Memcached (the slowest of all). I used the same ApacheBench test (ab -t 10 -c 10) and pointed the Memcache and Redis instances to the ones used by my Heroku app.

sorentwo commented 9 years ago

That's great to hear! I should quote that as a testimonial =)

monfresh commented 9 years ago

Hi Parker. Just following up to get a sense of when you plan to release a new version with these changes. Thanks!

sorentwo commented 9 years ago

This is a large enough behavior change that I'll be bumping the minor number. I'll probably be doing a release this next week.

sorentwo commented 9 years ago

@monfresh: v0.8.0 was just released, including the hiredis change and an overhaul of how redis is configured.