[Twisted-Python] twisted.internet.protocol.Protocol.connected flag purpose

Glyph Lefkowitz

2016-08-11 20:35:14 UTC

Post by Adi Roiban
Hi,
Looking at the code in trunk [1] I can see that
t.internet.protocol.Protocol will update the 'connected' attribute
upon makeConnection but will not update it on connectionLost
Is this the intended design?

I'm sure someone intended it at some point! But it seems like an extraneous wart. (This state can be detected by the presence of the 'transport' attribute, so it doesn't tell us anything interesting.)

Post by Adi Roiban
Is this design documented somewhere ?

Doubtful.

Post by Adi Roiban
I have check the narrative docs [2] and api-docs[3] but this attribute
is not documented.
What is the purpose of t.internet.protocol.Protocol.connected ?

Many things that date back this far are simple accidents of implementation. Protocol is, after all, from the very first versions of Twisted, before we even had TDD in place. So, unfortunately, as you're discovering, sometimes there are random little bits of maybe-useful-but-ultimately-broken functionality hanging around in uninspected parts of the codebase.

In all likelihood, whoever was implementing makeConnection (me probably, or maybe Itamar or Moshe) thought "hrm, maybe I'd like to know if the protocol is connected?", stuck the attribute on there, but then forgot that they'd had that idea by the time they got to connectionLost.

That said, I don't think we should bother removing this particular wart here. If we deprecate the attribute, we deprecate the use of the attribute 'connected' on any subclass of Protocol, which may be quite valid. Given that it's also an old-style class, it would add non-trivial overhead to attribute access too; it would generally be a big mess.

Instead, if you want to get rid of this, we should add a new way of writing Protocol objects, which is to use a new class decorator that fills out the methods on IProtocol with no-op implementations, rather than using inheritance. This new decorator, which resolves a bunch other problems with using inheritance, could drop accidental details like this one (if there are others; or just this one :)).

-glyph