Redis running on unix domain socket

[lordi]

There is no way to use django-websocket-redis with a redis instance running on a UNIX domain socket.

I think it can be fixed rather easily.

This issue had been reported before #66 but closed without a resolution.

[ppokrovsky]

I double. I solved it for now with certain hacks, but i'd prefer having a cleaner solution. Ultimately it goes down to use of ConnectionPool which does not support sockets. Is there a specific reason why RedisPublisher uses ConnectionPool instead of connection arguments in StrictRedis?

[jrief]

I actually never ran Redis listening on a UNIX socket. Always only on TCP/IP.

[ppokrovsky]

On the contrary I use sockets when possible.

But the question is why you decided to implement it with ConnectionPool instead of pure StrictRedis named params?

-- -P

[jrief]

As far as I remember, not for any specific reason. I think it was the sample code from Roberto.

[ppokrovsky]

I sent a pull request for this feature just in case

213

[lordi]

Awesome, it does the trick for me. Is there any way this PR would not be backwards compatible?

[ppokrovsky]

It should not break any backwards compatibility since this change by nature is kind of cosmetic.

[lordi]

One thing I'm worried about is connection re-use:

Redis-py provides a connection pool for you from which you can retrieve a connection. Connection pools create a set of connections which you can use as needed (and when done - the connection is returned to the connection pool for further reuse). Trying to create connections on the fly without discarding them (i.e. not using a pool or not using the pool correctly) will leave you with way too many connections to redis (until you hit the connection limit).

http://stackoverflow.com/a/31733749

[ppokrovsky]

That could be an issue with certain load yes, that's why I was asking if there was any specific reason behinf implementing it with CP in the first place. Ideally this behaviour should be configurable, through, e.g. 'use_connection_pool': True setting. But this will require a bit more effort than just changing few lines in Subscriber class. :)

[lordi]

Shouldn't be too complicated either, untested pseudo code:

from redis.connection import UnixDomainSocketConnection
if 'unix_socket_path' in settings.WS4REDIS:
    pool = redis.ConnectionPool(connection_class=UnixDomainSocketConnection,
                  path=settings.WS4REDIS['unix_socket_path'])
else:
     ..original code..

I can prepare a alternative PR later

[ppokrovsky]

I was going to give a feedback but, my, you were quick with that PR :)

Your solution is definitely more flexible and adds some sugar. Only from my taste it brings a bit of magic, when parameter name changes its face under the hood. Straightforward approach for me would be to inherit configuration parameters directly from redis library for each case: with and without connection pools and to let user decide whether to use connection pools or not (configuration-wide, like WS4REDIS_USE_CONNECTION_POOLS leaving defaults to true). Mostly because of explicity, but its a matter of taste really.

Disclaimer: above would add a tricky part with knowing which params to use in which case, but im guessing advanced users know what they are doing.

Not to start PR wars, its better to hear what @jrief have to say about it.