Opened 4 years ago

Closed 4 years ago

#5979 enhancement closed fixed (fixed)

Make `twisted.internet.reactor` import on Python 3

Reported by: itamar Owned by: itamar
Priority: normal Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/epollreactor-py3-5979-3
branch-diff, diff-cov, branch-cov, buildbot
Author: itamarst

Description (last modified by itamar)

Given the interleaved nature of twisted.internet and its tests, we probably can't port modules in isolation. Instead, we should first make the modules necessary for the basic reactor importable. Then when we port module by module, we can do so without having broken imports make it impossible.

Change History (9)

comment:1 Changed 4 years ago by itamarst

  • Author set to itamarst
  • Branch set to branches/epollreactor-py3-5979

(In [35687]) Branching to 'epollreactor-py3-5979'

comment:2 Changed 4 years ago by itamar

  • Description modified (diff)
  • Summary changed from Make `twisted.internet.epollreactor` import on Python 3 to Make `twisted.internet.reactor` import on Python 3

In order for this to be useful, we probably actually want twisted.internet.reactor to be importable, which slightly expands the scope of the ticket.

comment:3 Changed 4 years ago by itamarst

  • Branch changed from branches/epollreactor-py3-5979 to branches/epollreactor-py3-5979-2

(In [35711]) Branching to 'epollreactor-py3-5979-2'

comment:4 Changed 4 years ago by itamar

  • Keywords review added
  • Owner set to exarkun

Ready for review. I tried to minimize necessary changes; probably they could've been slightly more minimal. twisted.internet.reactor imports now.

Started test run:

comment:5 Changed 4 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to itamar
  1. Looks like the changes to can be dropped now
  2. Not new in this branch, but says "Linux: epoll(7) is the fault, since it scales well." which is correct in its way, but not what was intended I think.
  3. The udp and tcp imports now hidden inside reactor methods in should have comments to move back up to module scope once udp and tcp are ported
  4. The new SyntaxError for handling the unix import should have a similar comment, or unix should be handled the same way as udp and tcp (the latter is preferable, I think).
  5. Too bad about all of the extra changes. It's pretty sad that twisted.internet.task is a dependency of twisted.internet.reactor. That dependency seems to be due to twisted.protocols.tls? Now I wish I'd worked a little harder on that module. I want to ask for some ticket for fixing this, but I can't quite figure out what the ticket is.
  6. A comment about why there's a queue/Queue thing in would be nice
  7. Explaining what it means that things don't subclass Ephemeral anymore would probably be nice. I bet most people have no clue what this class is or what value it provided. Just an extra clause in the news fragment, like "; incorrectly pickling these types will no longer log a warning."
  8. Some conflicts in of course

Nothing too major here. Merge when the above are addressed.

comment:6 Changed 4 years ago by itamarst

  • Branch changed from branches/epollreactor-py3-5979-2 to branches/epollreactor-py3-5979-3

(In [35735]) Branching to 'epollreactor-py3-5979-3'

comment:7 Changed 4 years ago by itamar

1 and 8 were addressed by merge forward. 2, 3, 4, 6, 7 were addressed by 35737.

For IReactorUnix, I switched the check for whether to enable the interface to base it on the platformType, and moved imports in as with other tickets.

I opened ticket #6004 to deal with issue 5.

comment:9 Changed 4 years ago by itamarst

  • Resolution set to fixed
  • Status changed from new to closed

(In [35740]) Merge epollreactor-py3-5979-3.

Author: itamar Review: exarkun Fixes: #5979

Make a bunch of modules import on Python 3, port twisted.internet.default and .reactor to Python 3, and remove Ephemeral base class from a bunch of reactor-related classes.

Note: See TracTickets for help on using tickets.