#6077 enhancement closed fixed (fixed)

Port twisted.web.client.HTTPClientFactory to Python 3

Reported by: exarkun Owned by: itamar
Priority: normal Milestone: Python 3.3 Minimal
Component: web Keywords:
Cc: jknight Branch: branches/webclient-py3-6077-2
(diff, github, buildbot, log)
Author: itamarst, exarkun Launchpad Bug:

Description

This is the basis of the old web client.

Change History (11)

comment:1 Changed 22 months ago by DefaultCC Plugin

  • Cc jknight added

comment:2 Changed 22 months ago by exarkun

  • Author set to exarkun
  • Branch set to branches/webclient-py3-6077

(In [35999]) Branching to 'webclient-py3-6077'

comment:3 Changed 21 months ago by itamarst

  • Author changed from exarkun to itamarst, exarkun

(In [36411]) Branching to 'webclient-py3-6077'

comment:4 Changed 21 months ago by itamarst

(In [36412]) Move Agent tests into new test module. Refs #6077

comment:5 Changed 20 months ago by exarkun

  • Owner changed from exarkun to itamar

comment:6 Changed 20 months ago by itamarst

  • Branch changed from branches/webclient-py3-6077 to branches/webclient-py3-6077-2

(In [36434]) Branching to 'webclient-py3-6077-2'

comment:7 Changed 20 months ago by itamar

  • Keywords review added
  • Owner changed from itamar to exarkun

http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/webclient-py3-6077-2

The code in the new test_agent.py is identical to the tests for Agent that were previously in test_webclient.py, so not much to review there.

comment:8 Changed 20 months ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to itamar
  1. Doc for code parameter to Error.__init__ has a typo - bytesing
  2. It seems type check for code in Error.__init__ is untested. I deleted it and all the tests still pass. Also I didn't like the look of it, so hopefully it can just be deleted?
  3. in client.py, there seems to be a mix of two approaches to formatting bytes:
    • bytes + b'x' + bytes
    • networkString('%sx%s' % (nativeString(bytes), nativeString(bytes))

This isn't really a problem I guess, but it might be nice to decide on one approach and stick to it consistently. If there's some particular reason to prefer one or the other of these in different circumstances, please disregard (but maybe document it on the porting page?)

  1. I guess it would be good to have a ticket for dealing with the downloadPage test skips somehow. Delete downloadPage? I forget whether Agent is feature equivalent yet. Or... port downloadPage. And referenced from the code in the typical manner.
  2. Blech on testRawSomeCookies :(

That's it. If you can address all these (save the very last point) with nothing too surprising, please merge. Thanks.

comment:9 Changed 20 months ago by itamar

  1. Fixed.
  2. I think that means there's a missing test. It's not exactly important code though, so I'll just chalk it up to the large list of missing test coverage for twisted.web. Deleted.
  3. I switched to first format. My gut feeling is "use networkString variant if it's sufficiently complex that it's clearer than lots of addition", which these cases definitely aren't.
  4. #6197, added reference.

Left the icky test in, which I think you implied. Plus side, I think we're maybe one feature away (browser-style redirects) from being able to rewrite getPage and downloadPage as wrappers on top of Agent.

comment:10 Changed 20 months ago by itamarst

(In [36468]) Address review comments. Refs #6077

comment:11 Changed 20 months ago by itamarst

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

(In [36470]) Merge webclient-py3-6077-2.

Fixes: #6077
Author: itamar
Review: exarkun

Port twisted.web.client.getPage to Python 3. Along the way, moved Agent tests into their own test module.

Note: See TracTickets for help on using tickets.