IlyaSkriblovsky / txredisapi

non-blocking redis client for python twisted
Apache License 2.0
235 stars 91 forks source link

Disconnects do not return ConnectionError for lazy connections #112

Closed ysobolev closed 7 years ago

ysobolev commented 7 years ago

The documentation for lazy connections says that

If the connection with redis is lost, all commands will raise the ConnectionError exception, to indicate that there's no active connection. However, if the reconnect argument was set to True during the initialization, it will continuosly try to reconnect, in background.

My understanding is that even if reconnect is set to True, I should expect a ConnectionError. This would be useful in the case when it is important to reasonably quickly return an HTTP request with a failure if there are no available redis connections. However, instead the deferred never fires and txredisapi keeps trying to reconnect. For example, examples/twistedweb_server.py will not return an error to a GET request if there is no active redis connection.

IlyaSkriblovsky commented 7 years ago

I think docs are misleading about this point. reconnect=True is certainly coded to not raise any errors if no connections are active at the moment and is trying to make reconnects invisible to the end user. ConnectionError are only raised for commands that was sent before connection failure but not received responses until disconnected.

I think if you need response from Redis or failure within guaranteed period of time, you can't rely on ConnectionError anyway. Network connection might stall, server might be turned off, wire might be broken and you won't get any response (including failure) until TCP will drop connection by timeout. It seems to me that the only reliable way to guarantee response time is to do timing out on the client side. Deferred.addTimeout() from Twisted 16.5 can probably help.

ysobolev commented 7 years ago

Thanks! Makes sense.