Opened 4 years ago

Closed 21 months 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.
Last edited 22 months ago by Thijs Triemstra (previous) (diff)

comment:4 Changed 22 months ago by hawkowl

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

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

comment:5 Changed 22 months ago by hawkowl

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

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

comment:6 Changed 22 months ago by hawkowl

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

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

comment:7 Changed 21 months 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 21 months 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 21 months ago by hawkowl

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

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

comment:10 Changed 21 months 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.