Opened 5 years ago

Closed 3 years ago

#6177 enhancement closed fixed (fixed)

Port twisted.web.static to Python 3

Reported by: Itamar Turner-Trauring Owned by:
Priority: lowest Milestone: Python-3.x
Component: core Keywords:
Cc: Branch: branches/twstatic-py3-6177-4
branch-diff, diff-cov, branch-cov, buildbot
Author: hawkowl

Description

twisted.web.static should be ported to Python 3.

Attachments (1)

web.static-py3k-wip.patch (22.1 KB) - added by Ihar Hrachyshka 4 years ago.
First draft, not completed

Download all attachments as: .zip

Change History (11)

comment:1 Changed 4 years ago by Ihar Hrachyshka

Owner: set to Ihar Hrachyshka
Status: newassigned

Changed 4 years ago by Ihar Hrachyshka

Attachment: web.static-py3k-wip.patch added

First draft, not completed

comment:2 Changed 4 years ago by Ihar Hrachyshka

Owner: Ihar Hrachyshka deleted
Status: assignednew

I've mostly ported web.static and corresponding unit test to python3.x though it turned out the code depends on python.filepath (thru web.util), while filepath module does not seem to work ok in python3.x (even though corresponding porting ticket was closed as fixed).

For reference: web.util py3k ticket: #6178 unicode in filepath: #5203

comment:3 Changed 4 years ago by Ihar Hrachyshka

So after applying the patch, run-python3-tests fails while initially importing modules, as follows:

Traceback (most recent call last):

File "./admin/run-python3-tests", line 30, in <module>

unittest.main(module=None, argv=["run-python3-tests", "-v"] + testModules)

File "/usr/local/Cellar/python3/3.3.2/Frameworks/Python.framework/Versions/3.3/lib/python3.3/unittest/main.py", line 124, in init

self.parseArgs(argv)

File "/usr/local/Cellar/python3/3.3.2/Frameworks/Python.framework/Versions/3.3/lib/python3.3/unittest/main.py", line 168, in parseArgs

self.createTests()

File "/usr/local/Cellar/python3/3.3.2/Frameworks/Python.framework/Versions/3.3/lib/python3.3/unittest/main.py", line 175, in createTests

self.module)

File "/usr/local/Cellar/python3/3.3.2/Frameworks/Python.framework/Versions/3.3/lib/python3.3/unittest/loader.py", line 137, in loadTestsFromNames

suites = [self.loadTestsFromName(name, module) for name in names]

File "/usr/local/Cellar/python3/3.3.2/Frameworks/Python.framework/Versions/3.3/lib/python3.3/unittest/loader.py", line 137, in <listcomp>

suites = [self.loadTestsFromName(name, module) for name in names]

File "/usr/local/Cellar/python3/3.3.2/Frameworks/Python.framework/Versions/3.3/lib/python3.3/unittest/loader.py", line 96, in loadTestsFromName

module = import('.'.join(parts_copy))

File "/Users/ihrachyshka/proj/tw/twisted/twisted/web/test/test_static.py", line 19, in <module>

from twisted.web import static, http, script, resource

File "/Users/ihrachyshka/proj/tw/twisted/twisted/web/static.py", line 22, in <module>

from twisted.web.util import redirectTo

File "/Users/ihrachyshka/proj/tw/twisted/twisted/web/util.py", line 362, in <module>

class FailureElement(Element):

File "/Users/ihrachyshka/proj/tw/twisted/twisted/web/util.py", line 372, in FailureElement

loader = XMLFile(getModule(name).filePath.sibling(b"failure.xhtml"))

File "/Users/ihrachyshka/proj/tw/twisted/twisted/python/filepath.py", line 408, in sibling

return self.parent().child(path)

File "/Users/ihrachyshka/proj/tw/twisted/twisted/python/filepath.py", line 681, in child

newpath = abspath(joinpath(self.path, norm))

File "/usr/local/Cellar/python3/3.3.2/Frameworks/Python.framework/Versions/3.3/lib/python3.3/posixpath.py", line 92, in join

"components.") from None

TypeError: Can't mix strings and bytes in path components.

Version 0, edited 4 years ago by Ihar Hrachyshka (next)

comment:4 Changed 3 years ago by hawkowl

Author: hawkowl
Branch: branches/twstatic-py3-6177

(In [44195]) Branching to twstatic-py3-6177.

comment:5 Changed 3 years ago by hawkowl

Branch: branches/twstatic-py3-6177branches/twstatic-py3-6177-2

(In [44264]) Branching to twstatic-py3-6177-2.

comment:6 Changed 3 years ago by hawkowl

Branch: branches/twstatic-py3-6177-2branches/twstatic-py3-6177-3

(In [44382]) Branching to twstatic-py3-6177-3.

comment:7 Changed 3 years ago by hawkowl

Keywords: review added

Okay. This is a big patch, and I'm sorry!

Some considerations for reviewers:

  • This removes the use of Versioned. As it's pickling, this is an acceptable backwards-incompatible change.
  • File() handles both *encodings* and *content types* as str.
  • This contains an opportunistic port of twisted.web.util as bits of File requires it.
  • Parts that used os.path have been moved to using FilePath for py3 compat reasons.
  • Tests that leaked open files have been fixed.

Builders spun, please review.

comment:8 Changed 3 years ago by Wilfredo Sánchez Vega

Keywords: review removed

In compat.py:457: Return an list should be Return a list

In test_static.py:435: file = resource.openForReading() could be with resource.openForReading() as file: and eliminate the following file.close(). Same at several other points further down in the file. Also protects against raises in between, though in tests that's probably a minor thing.

Lots to like here, this is awesome even if you don't care about Py3. Thanks. Above comments are nits, don't need re-review if you fix them. You're good to go.

comment:9 Changed 3 years ago by hawkowl

Branch: branches/twstatic-py3-6177-3branches/twstatic-py3-6177-4

(In [44481]) Branching to twstatic-py3-6177-4.

comment:10 Changed 3 years ago by hawkowl

Resolution: fixed
Status: newclosed

(In [44483]) Merge twstatic-py3-6177-4: Port twisted.web.static to Python 3

Author: hawkowl Reviewer: wsanchez Fixes: #6177

Note: See TracTickets for help on using tickets.