On 31 March 2015 at 17:55, Demian Brecht <demianbrecht@gmail.com> wrote:

Hi all,

I'm not sure whether this should be python-list or here, but given it's a premature code review for http.client, I figured I'd post here first.

I'm in the process of adding support for chunked transfer encoding to http.client (issue #12319). One of the bits of functionality that I'm working on in is ironing out some of the kinks out in determining the content length of the request bodies. Given the number of data types allowed as bodies it does get a little messy, so I wanted to get some feedback here and see if anyone can shoot holes through it prior to updating the current patch. The tests are passing, but there may be use cases not accounted for in the new implementation.

https://gist.github.com/demianbrecht/f94be5a51e32bb9c81e1

The above is intended to replace _set_content_length (current state of the patch can be seen here: http://bugs.python.org/review/12319/diff/14331/Lib/http/client.py). There is a comprehensive list of data types currently supported can be found here: http://bugs.python.org/issue23740. Comments and feedback are much appreciated.

Nothing problematic leaps out at me here, but only if I look at your proposal in the context of the full patch. Your full patch does seem to have an edge case: for HTTP/1.1, if I pass a generator and no content-length you'll blow up by indexing into it. That's probably fine (your only option in this case would be to frame this request by half-closing the TCP stream, which I seem to recall you ruling out elsewhere), but you might want to wrap the exception in something more helpful: not sure. Cory