Opened 3 years ago

Last modified 3 years ago

#6698 enhancement new

Implement HostnameEndpoint.connect as an independent object with instance state

Reported by: ashfall Owned by: ashfall
Priority: normal Milestone:
Component: core Keywords: endpoint
Cc: Branch: branches/hostname-endpoint-fsm-6698
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun


As discussed in review point 5.6 of #4859, the implementation of HostnameEndpoint.connect should be refactored to be more like an explicit state machine:

Overall, I suspect connect could be implemented more simply - probably as an independent object with instance state (and perhaps as an explicit state machine) rather than a collection of nested functions with shared, closed-over mutable objects.

Change History (6)

comment:1 Changed 3 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/hostname-endpoint-fsm-6698

(In [42309]) Branching to 'hostname-endpoint-fsm-6698'

comment:2 Changed 3 years ago by glyph

I have mixed feelings about this. On the one hand, implementing things with machinist is probably a better way to go about it.

On the other hand, this would be our first required dependency under a more restrictive license than Python itself. This may reveal itself in ugly ways. For example, the FSF continues to spread FUD that GPL2 and Apache are incompatible, and, for example, Buildbot is GPL2-only forever. See here for more details. I don't think the Buildbot developers share FSF's interpretation, but since it's mutually-assured-destruction GPL instead of assignment-GPL then one litigious contributor can set off a land mine. (Of course, some Buildbot installs contain pyOpenSSL too, so what does that mean?)

The other dependencies which we're considering adding like this (service_identity, cffi) are all MIT licensed and don't have this problem. ZPL seems to my untrained eye to be BSD-equivalent as well, so while Zope really shouldn't have made their own license, it should be fine.

Opening this can of worms for an internal refactoring seems like a fairly high amount of pain for minimal gain. Any chance you could relicense machinist before the external-contributor contamination gets too significant to ever be able to do that? :-). Then I can start talking about technical problems instead ;-).

comment:3 Changed 3 years ago by glyph

Worse yet, I just discovered that machinist comes with a transitive hard dependency on Eliot, which is (A) also Apache-licensed, and (B) changing incompatibly on a regular enough basis to recommend pinning the dependency.

I think we need to block this change until we have had a discussion with interested parties about the impact of the license change. I'd rather Machinist just eliminate its hard dependency on Eliot (_FiniteStateLogger already appears to be nicely factored out; surely this could live in a separate module?) so we only have to talk about one package.

comment:4 follow-up: Changed 3 years ago by itamar

I'm hoping we can make a stable Eliot release sometime soon, but having Machinist not have a hard dependency on Eliot definitely seems reasonable.

comment:5 in reply to: ↑ 4 Changed 3 years ago by glyph

Replying to itamar:

I'm hoping we can make a stable Eliot release sometime soon, but having Machinist not have a hard dependency on Eliot definitely seems reasonable.

Cool, that does seem like the right call to me.

comment:6 Changed 3 years ago by glyph

Update on the licensing situation:

I think we can afford not to care, because the largest and most prominent example of GPL2/Twisted is clearly Buildbot, and pip install buildbot results in the following dependencies today:


As it happens, `sqlalchemy-migrate` is apache-licensed already. If even Buildbot doesn't care about this, it's unlikely it's worth the mental energy to care about it ourselves. Given Python's license gymnastics, it's not like when you install Twisted you're dealing with a pure-MIT stack anyway.

Note: See TracTickets for help on using tickets.