httpbakery .Client overly restrictive on body - io.ReaderSeeker

[roger peppe]

On 9 March 2016 at 03:03, Tim Penhey <tim.penhey at canonical.com> wrote:
> Hi folks,
>
> I came across this recently while looking at sending tools and charms
> between two controllers.
>
> The local storage interface gives us an io.ReaderCloser. Unfortunately,
> we can't give that to the http client for the POST method as it needs an
> io.ReaderSeeker.
>
> I'd like to challenge the need for the seek method.
>
> Is there any way we could change the method to take an io.Reader?

Unfortunately that's not possible in general because if the
request fails with an authorization error, it needs to acquire
the relevant credentials and try the request again, and
that's not possible unless it can go back to the start of the
body, because the client starts copying that immediately.

If we were doing the logic again, perhaps we might consider
using a 100-Continue header, but support for that has only
just arrived in Go 1.6 I'm very hand-wavy about how it might
work and whether it could work OK with all the proxies we
put in from of web servers.

> Perhaps with a documentation caveat that if it implements
> io.ReaderSeeker, that Seek(0,0) will be called.

That is indeed the case, and it is documented (although perhaps
not obviously enough):

    DoWithBody is like Do except that the given body is used for the body of
    the HTTP request, and reset to its start by seeking if the request is
    retried. It is an error if req.Body is non-zero.

Note "reset to its start" (i.e. Seek(0, 0))

> Right now, I'm going through a local temp file, but I'd rather not have
> to if I don't need to.

Might it be possible to change the interface defined by the storage interface
to a ReadSeeker? Or alternatively, if only some implementations support
seeking (for example I see that environs.fileStorageReader does, although
I'm not sure which actual storage type you're referring to), you could
do something like this: http://play.golang.org/p/H0qkxyf1cx

FWIW the blob store also supports seeking because it's backed by
mgo.GridFS which supports seeking,and we rely on this in the charmstore
to read zip files (the format requires seeking) directly from the blob
store.

How does that sound?

  cheers,
    rog.

On 9 March 2016 at 03:03, Tim Penhey <tim.penhey at canonical.com> wrote:
> Hi folks,
>
> I came across this recently while looking at sending tools and charms
> between two controllers.
>
> The local storage interface gives us an io.ReaderCloser. Unfortunately,
> we can't give that to the http client for the POST method as it needs an
> io.ReaderSeeker.
>
> I'd like to challenge the need for the seek method.
>
> Is there any way we could change the method to take an io.Reader?
>
> Perhaps with a documentation caveat that if it implements
> io.ReaderSeeker, that Seek(0,0) will be called.
>
> Right now, I'm going through a local temp file, but I'd rather not have
> to if I don't need to.
>
> Tim