Skip to content

Added the ability to inject redis clients into the Faye Redis engine. - #13

Open
suprememoocow wants to merge 5 commits into
faye:masterfrom
gitterHQ:inject-client
Open

Added the ability to inject redis clients into the Faye Redis engine.#13
suprememoocow wants to merge 5 commits into
faye:masterfrom
gitterHQ:inject-client

Conversation

@suprememoocow

Copy link
Copy Markdown

This PR allows the Redis client to be injected into the Faye Redis engine, instead of having it instantiated by the engine itself (using the host, port etc options).

Background

We're currently moving across to using Redis Sentinel. In node, we're using Node Redis Sentinel Client to coordinate our Redis connections. This library provides a client which walks like a Redis client and talks like a Redis client, but isn't a standard Redis client (as it maintains a connection to the Sentinel, listening for failover messages, etc).

Implementation

Two extra options have been added to the Faye Redis engine: client and subscriberClient. These are intended to be used together and as an alternative to specifying the Redis connection parameters host, port etc. They should each reference a Redis client (or in our case a Redis-client-like object).

@evantahler

Copy link
Copy Markdown

👍

This looks like it would work for the myriad of sentinel clients that exist (personally, I picked https://github.com/flyerhzm/redis-sentinel) and would also allow for simpler testing with https://github.com/hdachev/fakeredis as well

@evantahler

Copy link
Copy Markdown

bump?

@jcoglan

jcoglan commented Jun 2, 2014

Copy link
Copy Markdown
Collaborator

@evantahler Please don't do that, it's passive-aggressive. I have been having a really rough year and am trying to catch up on my OSS stuff. I'm getting to everything as soon as I can.

@suprememoocow

Copy link
Copy Markdown
Author

@jcoglan, sorry about the broken build on this PR. I've got no idea why redis seems to be failing on the travis build when running against Node 0.6. All the other builds (node 0.8, etc) pass. Any ideas?

@fatso83

fatso83 commented Sep 16, 2015

Copy link
Copy Markdown

For the record, 0.6 is no longer part of the Travis build, so it should pass the build checks now.

@puzrin

puzrin commented Feb 9, 2021

Copy link
Copy Markdown

IMO option createClient would be more flexible.

Also note:

  • multiple connection params are outdated, clients support URL (single string) now.
  • redis pkg dependency is not nice (imo should be dropped, because is weak peer dependency). People may wish ioredis.

@jcoglan what is status of this pakage? Can i help with refresh of outdated codebase? Or i should do the fork?

@fatso83

fatso83 commented Feb 10, 2021

Copy link
Copy Markdown

@puzrin I am not sure why you even bother asking. This package has not seen a single maintenance commit in seven years. James should have just archived the repo.

@puzrin

puzrin commented Feb 10, 2021

Copy link
Copy Markdown

@fatso83 it's not difficult to ask an be polite, before switch to gitterhq fork :)

@fatso83

fatso83 commented Feb 10, 2021

Copy link
Copy Markdown

@puzrin The GitterHQ has not seen any action in years either, so if you encounter any other issues, I would frankly look for an alternative. Still, if it works and scratches your itch, old software is good software :)

@puzrin

puzrin commented Feb 10, 2021

Copy link
Copy Markdown

@fatso83 fork has ancient look, but seems to solve all real problems. Of cause, i'd prefer to use mainstream and can prepare a set of commits. But need a minimal feedback from @jcoglan.

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