Ticket #6077 enhancement closed fixed

Opened 19 months ago

Last modified 17 months ago

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:


This is the basis of the old web client.

Change History


Changed 19 months ago by DefaultCC Plugin

  • cc jknight added


Changed 19 months ago by exarkun

  • branch set to branches/webclient-py3-6077
  • branch_author set to exarkun

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


Changed 17 months ago by itamarst

  • branch_author changed from exarkun to itamarst, exarkun

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


Changed 17 months ago by itamarst

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


Changed 17 months ago by exarkun

  • owner changed from exarkun to itamar


Changed 17 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'


Changed 17 months ago by itamar

  • owner changed from itamar to exarkun
  • keywords review added


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.


Changed 17 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.


Changed 17 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.


Changed 17 months ago by itamarst

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


Changed 17 months ago by itamarst

  • status changed from new to closed
  • resolution set to fixed

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