Opened 4 years ago

Closed 4 years ago

#6056 enhancement closed fixed (fixed)

Port twisted.names.client to Python 3

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: normal Milestone: Python 3.3 Minimal
Component: names Keywords:
Cc: Branch: branches/dns-client-py3-6056-2
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

Because it doesn't run on Python 3 now and the goal is for it to do so.

Change History (11)

comment:1 Changed 4 years ago by Jean-Paul Calderone

Milestone: Python 3.3 Minimal

comment:2 Changed 4 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/dns-client-py3-6056

(In [36043]) Branching to 'dns-client-py3-6056'

comment:3 Changed 4 years ago by Jean-Paul Calderone

Branch: branches/dns-client-py3-6056branches/dns-client-py3-6056-2

(In [36062]) Branching to 'dns-client-py3-6056-2'

comment:4 Changed 4 years ago by Jean-Paul Calderone

Keywords: review added
Owner: changed from Jean-Paul Calderone to Itamar Turner-Trauring

Porting occurred. There's also some twisted.names.root porting in this branch. Necessary for the same reason as hosts and cache, but I realized partway through that while it needs to import for the minimal milestone, it doesn't need to work, so I immediately stopped. Result is a slightly-more-than-necessary amount of porting in root.py and its tests. I left it out of the ported list.

Other stuff is the usual, zope.interface, bytes vs unicode, unpacking in parameter definitions, except syntax, etc. More conversions of str(Name) to Name.name to preserve bytesness. I had to add a bunch of missing test coverage for createResolver and I also deleted support for the deprecated Resolver.protocol attribute (the tests still used this deprecated attribute, so they needed changin').

Build results

comment:5 Changed 4 years ago by Itamar Turner-Trauring

I'll deal with this tomorrow, probably, but a note on root: it is possible (likely?) we will initially have setup.py installing only modules that are in the ported list. If a module that is required for code to run is not there, this will break things. I opened a ticket for porting twisted.python.win32 as a result, you should either do the same, or remove the dependency on root importing, or start thinking of alternate setup.py strategies. In the latter case you should open the ticket for setup.py porting to discuss this, since we need to do it real soon now.

comment:6 Changed 4 years ago by Jean-Paul Calderone

(In [36072]) Remove another deprecated, now broken, use of Resolver.protocol

refs #6056

comment:7 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: changed from Itamar Turner-Trauring to Jean-Paul Calderone

Hooray for more tests.

  1. getWarningMethod is unnecessary import now in client.py.
  2. Change news file to be removal, to note removal of deprecated Resolver.protocol.
  3. Some failures in the news test on Windows need to be fixed.
  4. In test_client.py, a typo: "A missing nameserver configuration file *is* results".
  5. test_names has at least two classes that are really testing client.py (FilterAnswerTests, RetryLogic), maybe others as well. Perhaps they should be moved to test_client.py for better Python 3 coverage?
  6. run-python3-tests doesn't pass either. Possibly you forgot a final commit? E.g. parseConfig still uses file() in version I'm seeing.

comment:8 Changed 4 years ago by Jean-Paul Calderone

Keywords: review added
Owner: changed from Jean-Paul Calderone to Itamar Turner-Trauring

Thanks. Sorry about the half-completed work.

  1. getWarningMethod is unnecessary import now in client.py.
  2. Change news file to be removal, to note removal of deprecated Resolver.protocol.
  3. In test_client.py, a typo: "A missing nameserver configuration file *is* results".

Fixed in r36080

  1. Some failures in the news test on Windows need to be fixed.

I punted, instead :/ Testing thread-using code is hard. I filed #6098 and marked the tests as skipped.

  1. test_names has at least two classes that are really testing client.py (FilterAnswerTests, RetryLogic), maybe others as well. Perhaps they should be moved to test_client.py for better Python 3 coverage?

Done in r36090 and r36091.

  1. run-python3-tests doesn't pass either. Possibly you forgot a final commit? E.g. parseConfig still uses file() in version I'm seeing.

Huh, probably got distracted by writing new tests and forgot to re-run them under Python 3. Fixed in r36087.

comment:9 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: changed from Itamar Turner-Trauring to Jean-Paul Calderone

Thanks!

It still looks like there's a couple of failed tests on Windows, unless I misread the build; skipping as a temporary solution is fine. Fix, then merge.

comment:10 Changed 4 years ago by Jean-Paul Calderone

(In [36094]) Skip more tests on Windows

refs #6056

comment:11 Changed 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [36097]) Merge dns-client-py3-6056-2

Author: exarkun Reviewer: itamarst Fixes: #6056

Port twisted.names.client (and twisted.names.resolve and some small bits of twisted.names.root) to Python 3. Also remove the deprecated Resolver.protocol attribute, remove some uses of it from Twisted, and add some missing test coverage.

Note: See TracTickets for help on using tickets.