Ticket #6025 enhancement closed fixed

Opened 8 months ago

Last modified 8 months ago

Port twisted.protocols.basic to Python 3

Reported by: itamar Owned by: itamar
Priority: normal Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/basic-py3-6025-3
Author: itamarst Launchpad Bug:

Description

twisted.protocols.basic should run on Python 3.

Change History

1

Changed 8 months ago by itamarst

  • branch set to branches/basic-py3-6025
  • branch_author set to itamarst

(In [35830]) Branching to 'basic-py3-6025'

2

Changed 8 months ago by itamar

  • keywords review added
  • owner set to exarkun
  • branch changed from branches/basic-py3-6025 to branches/basic-py3-6025-2

Ugh, for some reason the branch wasn't updated. Build running here:

 http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/basic-py3-6025-2

3

Changed 8 months ago by exarkun

  • owner changed from exarkun to itamar
  • keywords review removed

Thanks. Looks like you had some fun on this one.

Ugh, for some reason the branch wasn't updated. Build running here:

When this happens, please follow the directions.

  1. The test-case-name on twisted/protocols/basic.py needs to be updated
  2. The byte literals in the documentation for NetstringReceiver._LENGTH should be in C{}
  3. Lots of tests in the newly created twisted.protocols.test.test_basic don't follow the coding standard (misnamed, poorly written docstrings, use fail variation of assertion methods, etc). Please file a ticket for fixing them.
  4. I guess my general thoughts regarding _PY is that it should be used at top-level module scope and not anywhere else. Particularly the cases which aren't going to go away, like cases that deal with the differences between Python 2 str and Python 3 bytes, as in LineOnlyReceiverTestCase.test_buffer. Using it to define two alternate implementations of a helper function seems tolerable (and a function for getting an iterator over bytes that yields each individual byte as a bytes object instead of an integer sounds like one that will be used more than once anyway - new twisted.python.compat API?), but constantly re-checking it after import time bothers me, both for performance reasons and because of the tedium it introduces for debugging and overall cognitive load of a section of code. This approach would avoid worsening the Python 2 performance of the implementation of sendString, too.
    1. Other tests in this module that could use such a helper:
      1. LPTestCaseMixin.test_illegal
      2. NetstringReceiverTestCase.test_receiveNetstringPortions_3
      3. IntNTestCaseMixin.test_receive
      4. IntNTestCaseMixin.test_partial
  5. Something about new-style classes... NewStyleInt16TestCase is redundant on Python 3 I guess. Can we indicate that somehow, so eventually it's clear we can delete it?
  6. Sorry, I think the new comments are largely inappropriate - at least, as comments in the source. They would probably make a good labs blog post though.

4

Changed 8 months ago by itamarst

  • branch changed from branches/basic-py3-6025-2 to branches/basic-py3-6025-3

(In [35916]) Branching to 'basic-py3-6025-3'

5

Changed 8 months ago by itamar

  • owner changed from itamar to exarkun
  • keywords review added

Addressed all review comments. I skipped the newstyle tests, added utility method for netstring formatting, and sanitized the comment about Python 3.

Started  http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/basic-py3-6025-3

6

Changed 8 months ago by exarkun

  • owner changed from exarkun to itamar
  • keywords review removed
  1. I guess there should be a .removal news fragment, for the removal of writing non-str.
  2. LPTestCaseMixin.test_illegal still has a subjective comment about Python 3, and some code that would be simplified by iterbytes.

That's all. Please merge once those are addressed.

7

Changed 8 months ago by itamarst

  • status changed from new to closed
  • resolution set to fixed

(In [35925]) Merge basic-py3-6025-3.

Author: itamar Review: exarkun Fixes: #6025

twisted.protocols.basic now works on Python 3. Also you can't send arbitrary objects using NetstringReceiver.sendString (deprecated since v10.0).

Note: See TracTickets for help on using tickets.