weixin_39798031
weixin_39798031
2020-12-25 17:28

add optional 4th param for Redis auth support


$ 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 ( https://github.com/mranney/node_redis/issues/155 ): test_jobs uses 'ready' and current versions of node_redis fail due to an auth issue.

Thank you. - jeremiah

该提问来源于开源项目:andyet/thoonk.js

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

4条回答

  • weixin_39798031 weixin_39798031 4月前

    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

    点赞 评论 复制链接分享
  • weixin_39798031 weixin_39798031 4月前

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

    点赞 评论 复制链接分享
  • weixin_39798031 weixin_39798031 4月前

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

    点赞 评论 复制链接分享
  • weixin_39566578 weixin_39566578 4月前

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

    点赞 评论 复制链接分享

相关推荐