Code review comment for lp:~jameinel/goose/transfer-content-length-1124561

Revision history for this message
Dave Cheney (dave-cheney) wrote :

LGTM. I can't believe that bug in nginx took SO LONG to get fixed.

On Sun, Jun 23, 2013 at 7:17 PM, John A Meinel <email address hidden>wrote:

> The proposal to merge lp:~jameinel/goose/transfer-content-length-1124561
> into lp:goose has been updated.
>
> Description changed to:
>
> http/client: Set Content-Length
>
> The net/http.Request object takes a Reader
> so you have to set ContentLength as an attribute
> or it will not set in in the POST request.
>
> Fixes bug #1124561.
>
> Oddly enough, we always buffer everything anyway,
> (probably to handle retries?). I would have thought
> for at least BinaryRequest we would have not
> touched the io.Reader that we got.
>
> https://codereview.appspot.com/10465043/
>
>
> For more details, see:
>
> https://code.launchpad.net/~jameinel/goose/transfer-content-length-1124561/+merge/170976
> --
>
> https://code.launchpad.net/~jameinel/goose/transfer-content-length-1124561/+merge/170976
> Your team juju hackers is requested to review the proposed merge of
> lp:~jameinel/goose/transfer-content-length-1124561 into lp:goose.
>

« Back to merge proposal