Make S3 (and other libraries) take an interface for HTTP client

[stevvooe]

Right now, we are using the s3 library and it is making a new connection for each request, leading to performance issues. The main issue is that a new transport is created each time a method is called on S3. The dialer implementation here is probably the source of a lot of problems seen with this library, as well. Compare that with the net/http.DefaultTranport configuration.

By default, the library should just leverage the default transport. Let's par down the cleverness and just allow an http.Client to be passed into s3.New. Remove the ConnectTimeout and ReadTimeout from the S3 struct, as well. These are breaking the abstraction.

Ideally, these libraries should just take an interface like this:

type HTTPClient struct {
    Do(req *Request) (resp *Response, err error)
}

If you are amenable to this approach, we can submit a PR.

[alimoeeny]

Thanks @stevvooe .
Definitely.
Would it break people's code? I mean would it require people to change the way they are using the S3 library? If so, it worth keeping the backward compatibility?

[stevvooe]

@alimoeeny We generally vendor everything, so I am not worried about broken builds. It seems like maintaining backwards compatibility should be mostly possible, except for the broken timeout configuration options (since they will be ignored).

Let me know how you'd like to proceed.

[alimoeeny]

Fair enough, @stevvooe
As long as people can still define their timeout, through the client / transport they pass in. It looks good to me.
In the past there has been discussions about different needs for different timeouts for different use cases.
Looking forward to your PR.

[stevvooe]

@alimoeeny Further research shows that the 30ms connection overhead isn't the biggest bottleneck when making round trips to s3. I'm not sure if it changes this ticket but it does affect the priority of the fix.