On Aug 11, 2016, at 7:21 AM, Adi Roiban <adi@roiban.ro> wrote:

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.)

Is this design documented somewhere ?

Doubtful.

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