cache_store connection url and default options #166

Closed
wants to merge 1 commit into
from

10 participants

@prashantrajan

The cache_store example with an extra options hash at the end does not seem to work as listed in the example at https://github.com/jodosha/redis-store/blob/master/redis-rails/README.md#cache-store:

# config/application.rb
config.cache_store = :redis_store, "redis://localhost:6380/10/cache", { expires_in: 90.minutes }

I've added a failing test that shows that when trying to specify a single server with options (e.g. :expires_in), it ends up creating a Redis::DistributedStore with an additional connection to a non-existent redis://127.0.0.1:6379

Relates to #139 & #140. The response there was to use :servers / :redis_server but that seems to only apply to the Rails session store, not the cache_store.

I've also added another test that shows that the default options to the store (e.g. expires_in) don't get passed down to the write calls when an explicit :expires_in option is not given to the write calls. Not sure if this is meant to be supported but the example in the Readme implies it does.

@JackCA

+1 to this. Ran into it today and took a while to figure out what was going on.

@nevir

Ran into this as well

@mjtko mjtko added a commit to mjtko/redis-store that referenced this pull request Feb 20, 2013
@mjtko mjtko Extract options before creating connection
Calls extract_options! on the addresses arguments before passing the addresses to the ::Redis::Factory.  This fixes #166 so default cache options can now be passed without causing a DistributedStore to be created with both the intended server as well as a spurious localhost connection. 
e6437d8
@mjtko mjtko added a commit to mjtko/redis-store that referenced this pull request Feb 20, 2013
@mjtko mjtko Extract options before creating connection
Calls extract_options! on the addresses arguments before passing the addresses to the ::Redis::Factory.  This fixes #166 so default cache options can now be passed without causing a DistributedStore to be created with both the intended server as well as a spurious localhost connection. 
ae45c84
@mjtko

I have also run into this. Fixing the Store initializer to extract_options! first seems to do the job and such a fix is now open as PR #169.

@mjtko mjtko added a commit to mjtko/redis-store that referenced this pull request Feb 26, 2013
@mjtko mjtko Extract options before creating connection
Calls extract_options! on the addresses arguments before passing the addresses to the ::Redis::Factory.  This fixes #166 so default cache options can now be passed without causing a DistributedStore to be created with both the intended server as well as a spurious localhost connection. 
f637e56
@austinthecoder austinthecoder pushed a commit that referenced this pull request Mar 20, 2013
Austin Schneider Calls extract_options! on the addresses arguments before passing the …
…addresses to the ::Redis::Factory. This fixes #166 so default cache options can now be passed without causing a DistributedStore to be created with both the intended server as well as a spurious localhost connection.
16616fe
@JackCA

It's tough to track down the status of this bug across so many issues. Does anyone know what the blocker is for the merge? Looks like @freegenie has made a few attempts at addressing this and more related issue

@rickmzp

ran into this issue as well. looking forward to fix!

@freegenie

@rickmzp this one should be fixed on master now, can you please try to use master and see if the problem goes away?

@jodosha
redis-store member

@freegenie @JackCA @rickmzp Is the issue fixed for you?

@rgravina

I've finding this issue still exists on master for me. I'm initialsing the session store like this:

uri = URI.parse(Rails.configuration.redis.url)
MyApplication::Application.config.session_store :redis_store, key: 'my_key', servers: {host: uri.host, port: uri.port, password: uri.password, namespace: 'my_namespace', expires_in: 1.month}

I have also tried the alternate url syntax

MyApplication::Application.config.session_store :redis_store, key: 'my_key', redis_server: Rails.configuration.redis.url, namespace: 'my_namespace', expires_in: 1.month

I'm still seeing -1 on TTL for session keys... am I doing anything wrong? Note that I'm using https://github.com/redis-store/redis-rails which depends on redis-store 1.1.4.

@rgravina

I've noticed that the issue only occurs when a namespace is provided as an option. I'm using the connect URL as a workaround, like so:

MyApplication::Application.config.session_store :redis_store, :key =>'my_key', :redis_server => "redis://url_to_redis:port/0/my_namespace", :expires_in => 1.month
@benilovj

My testing suggested that this issue was fixed with redis-activesupport 3.2.4; I used the following command on the rails console to debug:

ActiveSupport::Cache::RedisStore.new("redis://127.0.0.1:6380/1", { :expires_in => 1.second }).instance_variable_get(:@data).class

It returns Redis::DistributedStore before the fix and should return Redis::Store afterwards.

@benilovj benilovj added a commit to alphagov/support that referenced this pull request Jan 2, 2014
@benilovj benilovj Bump the redis-rails gem
Before this change, this app was running with a patched version
of the redis-activesupport gem which fixed a pretty nasty bug (
redis-store/redis-store#166). This bug
was fixed upstream (redis-store/redis-store#190)
and released with redis-activesupport 3.2.4 so we don't need to
maintain a separate branch any more.
d08f00e
@benilovj benilovj referenced this pull request in alphagov/support Jan 2, 2014
Merged

Bump the redis-rails gem #118

@radar
redis-store member

Please open a new PR if this still an issue.

@radar radar closed this Feb 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment