Opened 4 years ago

Closed 4 years ago

#6077 enhancement closed fixed (fixed)

Port twisted.web.client.HTTPClientFactory to Python 3

Reported by: Jean-Paul Calderone Owned by: Itamar Turner-Trauring
Priority: normal Milestone: Python 3.3 Minimal
Component: web Keywords:
Cc: jknight Branch: branches/webclient-py3-6077-2
branch-diff, diff-cov, branch-cov, buildbot
Author: itamarst, exarkun

Description

This is the basis of the old web client.

Change History (11)

comment:1 Changed 4 years ago by DefaultCC Plugin

Cc: jknight added

comment:2 Changed 4 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/webclient-py3-6077

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

comment:3 Changed 4 years ago by itamarst

Author: exarkunitamarst, exarkun

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

comment:4 Changed 4 years ago by itamarst

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

comment:5 Changed 4 years ago by Jean-Paul Calderone

Owner: changed from Jean-Paul Calderone to Itamar Turner-Trauring

comment:6 Changed 4 years ago by itamarst

Branch: branches/webclient-py3-6077branches/webclient-py3-6077-2

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

comment:7 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review added
Owner: changed from Itamar Turner-Trauring to Jean-Paul Calderone

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 4 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to Itamar Turner-Trauring
  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 4 years ago by Itamar Turner-Trauring

  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 4 years ago by itamarst

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

comment:11 Changed 4 years ago by itamarst

Resolution: fixed
Status: newclosed

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