#6092 enhancement closed fixed (fixed)

Port twisted.names.hosts to Python 3

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

Description

This is (sneakily) needed by twisted.names.client (createResolver imports it).

Change History (6)

comment:1 Changed 23 months ago by exarkun

  • Author set to exarkun
  • Branch set to branches/hosts-py3-6092

(In [36050]) Branching to 'hosts-py3-6092'

comment:2 Changed 23 months ago by exarkun

  • Keywords review added
  • Owner changed from exarkun to itamar

That was relatively straightforward. Build results.

comment:3 Changed 23 months ago by itamar

  • Keywords review removed
  • Owner changed from itamar to exarkun

Looks good. Some issues:

  1. Missing __future__.
  2. You're passing unicode to FilePath on Python 3, which isn't really supported in theory, but works in practice. Maybe that's OK though, and it does make things easier for users of the library. Perhaps we should have a ticket for "FilePath can open unicode paths even if it can't yet do child() etc."? Or we can change this to byte paths.

Fix (or not, if you're ok with unicode paths), then merge.

comment:4 Changed 23 months ago by exarkun

  • Status changed from new to assigned

Thanks!

Missing __future__.

Fixed in r36056.

You're passing unicode to FilePath on Python 3, which isn't really supported in theory, but works in practice.

It's a short branch, I know, but it would make the review comment a lot easier to understand if you identified what part of the code you're talking about. Initially I thought I had forgotten to do some encoding in the new test helper, GoodTempPathMixin, then I saw that I hadn't forgotten to do it so I thought you had misread the diff. Then I thought maybe there was some code somewhere else I'd forgotten about so I grepped the diff. I didn't see any use of FilePath with unicode, but I noticed hosts.py importing FilePath. Then I re-examined hosts.py for FilePath uses and noticed there are some places where unicode really is passed to it - whenever the default value for hosts.Resolver.__init__'s file parameter is used. Perhaps that's what this comment is about? I'll fix that and add a test, too - which will fail, because using unicode with FilePath doesn't currently work. ;)

comment:5 Changed 23 months ago by itamar

Resolver.__init__, yes but not only. I was less specific (and thought it worked) because I thought you were also passing in unicode in the tests. So I also misread the diff in addition to catching the real problem.

comment:6 Changed 23 months ago by exarkun

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

(In [36059]) Merge hosts-py3-6092

Author: exarkun
Reviewer: itamarst
Fixes: #6092

Port twisted.names.hosts to Python 3.

Note: See TracTickets for help on using tickets.