Skip to content
This repository has been archived by the owner on Jan 10, 2019. It is now read-only.

add optional 4th param for Redis auth support #15

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

mrtidy
Copy link

@mrtidy mrtidy commented Dec 29, 2011

$ grep ^requirepass /etc/redis/redis.conf 
requirepass booman

For this case, if I only provide Thoonk with hostname and port, then I get an error from RedisClient:

Error Ready check failed: Error: ERR operation not permitted

I verified that existing tests all still pass as well as possible but I haven't committed the modifications of the tests supporting passwords because of a node_redis issue ( redis/node-redis#155 ): test_jobs uses 'ready' and current versions of node_redis fail due to an auth issue.

Thank you.

  • jeremiah

@mrtidy
Copy link
Author

mrtidy commented Dec 30, 2011

I just added a second commit to this request. The documentation in README.md is currently wrong. It left off the error parameter in the callback of feed.publish and has the order of the parameters for feed.retract backwards.

current, incorrect docs:

feed.publish('item contents', 'optional id', function(item, id) {
    //done publishing!
});
...
feed.retract('item id', function(id, error) {
    //success?
});

corrected docs:

feed.publish('item contents', 'optional id', function(err, item, id) {
    //done publishing!
});
...
feed.retract('item id', function(err, id) {
    //success?
});

I renamed error in the retract callback to err to be consistent with other docs in this file.

Thank you.

  • jeremiah

@mrtidy
Copy link
Author

mrtidy commented Jan 13, 2012

Are there any doubts with this diff? I'm happy to adjust if necessary.

@mrtidy
Copy link
Author

mrtidy commented Apr 15, 2012

It would be helpful if you would merge in this pull.

@mcfazeli
Copy link

I also implemented authentication, but on master, I hadn't noticed your pull request before. It's a straightforward change.

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

Successfully merging this pull request may close these issues.

None yet

2 participants