Opened 2 years ago

Closed 2 years ago

#6056 enhancement closed fixed (fixed)

Port twisted.names.client to Python 3

Reported by: exarkun Owned by: exarkun
Priority: normal Milestone: Python 3.3 Minimal
Component: names Keywords:
Cc: Branch: branches/dns-client-py3-6056-2
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

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 2 years ago by exarkun

  • Milestone set to Python 3.3 Minimal

comment:2 Changed 2 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/dns-client-py3-6056

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

comment:3 Changed 2 years ago by exarkun

  • Branch changed from branches/dns-client-py3-6056 to branches/dns-client-py3-6056-2

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

comment:4 Changed 2 years ago by exarkun

  • Keywords review added
  • Owner changed from exarkun to itamar

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 2 years ago by itamar

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 2 years ago by exarkun

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

refs #6056

comment:7 Changed 2 years ago by itamar

  • Keywords review removed
  • Owner changed from itamar to exarkun

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 2 years ago by exarkun

  • Keywords review added
  • Owner changed from exarkun to itamar

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 2 years ago by itamar

  • Keywords review removed
  • Owner changed from itamar to exarkun

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 2 years ago by exarkun

(In [36094]) Skip more tests on Windows

refs #6056

comment:11 Changed 2 years ago by exarkun

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

(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.