Opened 4 years ago

Closed 4 years ago

#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
branch-diff, diff-cov, branch-cov, buildbot
Author: itamarst, exarkun


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 exarkun

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

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

comment:3 Changed 4 years ago by itamarst

  • Author changed from exarkun to itamarst, 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 exarkun

  • Owner changed from exarkun to itamar

comment:6 Changed 4 years 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 4 years ago by itamar

  • Keywords review added
  • Owner changed from itamar to exarkun

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

comment:8 Changed 4 years 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, 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

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