Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

master always create new redis instance #229

Closed
wants to merge 1 commit into from
Closed

master always create new redis instance #229

wants to merge 1 commit into from

Conversation

miahabdu
Copy link

No description provided.

@miahabdu miahabdu closed this May 17, 2015
@dchacke
Copy link
Owner

dchacke commented May 18, 2015

@miahabdu Hi there! Even though you closed this already, I am curious what your reasoning behind this was - maybe it makes sense and we can use it for the project. Let me know :)

@miahabdu
Copy link
Author

Hi @dchacke! I recently migrated my app to AWS and created an elasticache redis cluster. The problem that I was facing is that somewhere in my app (I think a gem) was defining the REDIS constant with the localhost address, even though I set my REDIS_URL in my EC2 instance to point to my elasticache cluster. This was causing the websocket handshake to fail because it couldn't find a redis instance to connect to, and wasn't picking up my REDIS_URL environment variable that I defined because it was being overwritten with the constant variable. (The ruby redis client automatically defaults to the redis url defined in the REDIS_URL environment variable when you call Redis.new if you don't supply it any parameters).

So by refactoring how the redis gets instantiated with the changes in this pull request helped me get over the issue I described above. It could be useful to change the code to what this pull request suggests, but I'll leave that to your discretion.

P.S. Thanks for your work on this gem. Super useful.

@dchacke
Copy link
Owner

dchacke commented May 26, 2015

@miahabdu Hmmm I guess it would be hard to override the other gem's functionality... If you would like, you could send a PR allowing for some config options we could put in an initializer in which we could explicitly set the Redis instance we want to use.

Something like:

# config/initializers/entangled.rb

Entangled.config do |config|
  config.redis = $redis # or something similar
end

The implementation of that method could look something like this:

def config
  yield self
end

def redis=(redis_instance)
  @redis = redis_instance
end

Or something similar. Not quite sure where the best place to put this would be.

I'll also put this in the todo list in the Readme since it would be helpful for further configuration options in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants