Opened 2 years ago

Closed 13 months ago

Last modified 13 months ago

#6065 enhancement closed fixed (fixed)

Port twisted.internet.base to Python 3

Reported by: itamar Owned by: itamar
Priority: normal Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/base-py3-6065-2
(diff, github, buildbot, log)
Author: itamarst Launchpad Bug:

Description

twisted.internet.base should run on Python 3.

Change History (15)

comment:1 Changed 2 years ago by itamarst

  • Author set to itamarst
  • Branch set to branches/base-py3-6065

(In [35978]) Branching to 'base-py3-6065'

comment:2 Changed 2 years ago by itamar

  • Keywords review added
  • Owner set to exarkun

comment:3 Changed 2 years ago by itamar

You'll want the 2nd build. Sigh.

comment:4 Changed 2 years ago by itamar

  • Keywords review removed

Taking it back from review, discovered another relevant test module.

comment:5 Changed 2 years ago by itamar

  • Keywords review added

comment:6 Changed 2 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to itamar

Thanks.

  1. DelayedCallTests in twisted.test.test_internet belongs somewhere else I think. twisted.internet.test.test_time? Or twisted.internet.test.test_base (only because, perhaps, it's something of a stretch for an interface test).
  2. What about twisted.internet.test.test_base? That seems like an important part of porting twisted.internet.base...
  3. Some of the process code in twisted.internet.base looks like it won't work on Python 3 - it uses unicode but doesn't import twisted.python.compat.unicode. Perhaps that's fine for now, since porting process support does seem like a fairly separate task. So maybe this ticket is just "port some of twisted.internet.base".
  4. Maybe TestProducer._dontPausePullConsumerTest should have a docstring now, at least documenting the allowed values for methodName.

comment:7 Changed 2 years ago by itamarst

(In [36027]) Address review comments. Refs #6065

comment:8 Changed 2 years ago by itamar

  • Keywords review added
  • Owner changed from itamar to exarkun
  1. That already had a test for this, so just deleted one I added.
  2. Oops. Added.
  3. I updated the test description to note that it doesn't include process code, and I reverted the changes I made to process code in base.
  4. Added.

Ready for review, I think:
http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/base-py3-6065

comment:9 Changed 2 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to itamar
  1. The pass in the body of nothing in test_base.py is unneeded. Hooray docstrings.
  2. There's a merge conflict in the howto.

Otherwise looks good. Please merge when these are addressed.

comment:10 Changed 2 years ago by itamarst

  • Branch changed from branches/base-py3-6065 to branches/base-py3-6065-2

(In [36077]) Branching to 'base-py3-6065-2'

comment:11 Changed 2 years ago by itamarst

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

(In [36081]) Merge base-py3-6065-2.

Author: itamar
Review: exarkun
Fixes: #6065

Port twisted.internet.base to Python 3.

comment:12 Changed 13 months ago by garrison

  • Resolution fixed deleted
  • Status changed from closed to reopened

I don't believe that twisted.internet.base is completely ported to Python3. In particular, there are mentions of str and unicode in the file that correspond to Python2 usage.

comment:13 Changed 13 months ago by itamar

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

I don't think we're going to back out this branch at this point, though. I'm going to close this and open a new ticket.

comment:14 Changed 13 months ago by exarkun

To elaborate on itamar's comment somewhat:

Thanks for your interest in this area of Twisted, garrison.

The workflow used for the ticket tracker is to only re-open fixed tickets if the corresponding change to trunk is also being reverted.

Given how long has elapsed since this change was made (it has been included in releases, later changes have been based on it, etc) it's not likely we'll revert the code.

Please file new tickets specifically describing the remaining issues (unless itamar does it for you).

Thanks again.

comment:15 Changed 13 months ago by garrison

Thanks, makes sense.

Note: See TracTickets for help on using tickets.