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

Adding auth to the redis store. #551

Closed
wants to merge 1 commit into from
Closed

Conversation

phillro
Copy link

@phillro phillro commented Oct 1, 2011

Hi. I added auth to the redis store.

thx.

@3rd-Eden
Copy link
Contributor

3rd-Eden commented Oct 5, 2011

Few comments on this, you really shouldn't throw an error when it fails, but just emit an error event for the store.
Also, can you stick to our code style? See https://github.com/LearnBoost/socket.io/wiki/Contributing (style guide section) on a small overview of it.

@dshaw
Copy link
Contributor

dshaw commented Oct 16, 2011

-1

There are 2-3 important configuration parameters for running Redis in production. When I rewrote this, I chose this minimal configuration internally, which is already pretty noisy and repetitive, and set it up you coud easily pass in a fully configured RedisClient. Furthermore, the API for auth has changed quite a few times in node_redis and I'd rather not track this internally. The baseline parameters that we support internally have been stable for at least over a year, if not for the life of the project.

These essential items are:

  1. and on error listener - or your entire process will exit if there's an issue with redis.
  2. auth
  3. optionally, db selection

new RedisStore({ pub: ..., sub: ..., cmd: ... })

@dshaw
Copy link
Contributor

dshaw commented Oct 20, 2011

Oops, I was just chatting with @3rd-Eden and and I realized I referenced the internal property names in the post above.
Should be: new RedisStore({ redisPub: redisPub, redisSub: redisSubscriber, redisClient: redisClient }))

@diversario
Copy link

So, in case where auth is required - how would I do that here?

@dshaw
Copy link
Contributor

dshaw commented Jan 10, 2012

@diversario Configure your instance of RedisClient externally and pass in the instance as indicated above.

var redis = require('redis')
  , sio = require('socket.io')
  , RedisStore = sio.RedisStore
  , io = sio.listen();

var port = 6379
  , hostname = 'redis.dshaw.com'
  , password = 's3cret'
  , db = 8;

// redisClient (can be the same as redisPub)
var redisClient = redis.createClient(port, hostname);
redisClient.auth(password, function (err) { if (err) throw err; );

var redisSubscriber = redis.createClient(port, hostname);
redisSubscriber.auth(password, function (err) { if (err) throw err; });

io.set('store', new RedisStore({ redisPub: redisClient, redisSub: redisSubscriber, redisClient: redisClient })));

@diversario
Copy link

Ah, that makes sense, thank you.

@dshaw
Copy link
Contributor

dshaw commented Jan 10, 2012

np.

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.

5 participants