BAD SMELL in overriding redis connection settings #147

Closed
behrad opened this Issue Nov 6, 2012 · 1 comment

1 participant

@behrad

We are accessing a remote redis instance from kue, When overriding ' kue.redis.createClient ' your node app should have access to 'redis' module to ' redis = require('redis') ' so a standalone redis module should be installed! (this way hiredis optional dependency of redis module is not installed by default) This caused us to hardly debug a problem with the redis client's javascript parser. (We thought we are using the internal kue's redis module which contains hiredis parser, but we were not)

@LearnBoost The first day I saw the way I should define redis connection settings, I felt bad about the way I should override ' kue.redis.createClient ' I think kue should not expose 'redis' module to it's caller and make them just depend to kue. I'd like to propose two simple fast alternatives one creating a simple constructor for kue to set the redis config, and later calling a template method in kue.redis.createClient which returns the config and should be overridden by kue callers if they want to change it.

kue.redis.getConnectionSettings = function() {
   return {
      host: '2.2.2.2', port: 1234, user: '', password: ''
   };
};
@behrad behrad referenced this issue Nov 6, 2012
Closed

No job.data !! #144

@ghost

I totally agree with @behrad in that this override method of creating the connection is quite bad. It not only causes dependency problems. But also, it makes it hard to create 2 queues connected to 2 separate redis instances, since the connection details are captured in the closure and can't be changed, and @behrad's solution doesn't solve this either. I think the best option would be to let createQueue accept the same 2 parameters as redis.createConnection()

@behrad behrad closed this Jan 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment