[Twisted-Python] stop/start client connections with loseConnection in ReconnectingClientFactory

[Glyph]

> On Mar 26, 2019, at 12:14 AM, Chris Withers <chris at withers.org> wrote:
> 
> On 24/03/2019 04:30, Glyph wrote:
>> I'd further note that ClientService is /generally/ the new, good way to do things and ReconnectingClientFactory is the old, bad way.  Our hope is to eventually deprecate ReconnectingClientFactory and most of the APIs that it uses, but this is a big project that we have not been able to make much progress on in the last, ahem, decade or so.
> 
> What's the big advantage(s) of ClientService over ReconnectingClientFactory?
> 
> The Autobahn guys still show ReconnectingClientFactory in their docs, and I remember looking at this before and ending up going with ReconnectingClientFactory because it works and didn't look like it'd need much effort to integrated into an existing code base.

Let me count the ways.

ReconnectingClientFactory is destined for deprecation, eventually.  You should just adopt the "new" thing now so that if we get more energy to cycle the APIs and delete the old stuff, you'll have less hassle to deal with.  ("New" is in quotes here since it's been around for well over 3 years at this point; Autobahn should update too, not just you.)
ClientService works with endpoints, which means you can use it with any kind of transport, like SSH transports, subprocesses, etc.  Most practically, it works with HostnameEndpoint which is a much better way to get TLS than connectSSL; ReconnectingClientFactory works, kind of accidentally, with TLS since connectSSL is on the reactor, but it won't use happy eyeballs and it won't connect over IPv6, so connections will be slower and less reliable.
Conceptually, ClientService has a much clearer and more useful responsibility: its job is to maintain a state (i.e.: that there is a single connection, that it is connected) rather than to do a thing.  For example, 
if you want to shut down a ReconnectingClientFactory:
you have to call stopTrying, then uh...
find the last protocol it built with buildProtocol, then
grab its transport (hope it's saving that transport as '.transport', because it doesn't actually have to) 
call loseConnection
remember to trap connectionLost so you can see when its done.
if you want to shut down a ClientService
call stopService
wait for the Deferred that it returned to fire
ClientService is (mostly) implemented using composition rather than inheritance, so much less of the guts of the internal implementation is hanging around where you might accidentally twiddle it and break its assumptions, so you can trust its guarantees more.
other benefits of composition: you don't have to override attributes of your Protocol and thereby indulge in subclassing yourself to get notifications; consider 'prepareConnection', 'whenConnected'.
the retry policy mechanics are better documented and much easier to customize
it's backed by a formal state machine - not that I'm aware of any specific bugs in ReconnectingClientFactory but do you think it got all of these state transitions correct: https://gist.github.com/glyph/614be03151556333efe04b849fa05930 <https://gist.github.com/glyph/614be03151556333efe04b849fa05930>
It's more testable because it just takes its clock and reactor as constructor parameters, rather than requiring post-hoc poorly-documented attribute patching to become testable.

Hopefully at least some of this is convincing :)

-g
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20190326/143f0701/attachment-0002.html>