Code review comment for lp:~gz/juju-core/1.16_ssl_verification_bootstrap_state_1268913

Revision history for this message
Martin Packman (gz) wrote :

Please take a look.

https://codereview.appspot.com/56560043/diff/20001/environs/httpstorage/storage.go
File environs/httpstorage/storage.go (right):

https://codereview.appspot.com/56560043/diff/20001/environs/httpstorage/storage.go#newcode26
environs/httpstorage/storage.go:26: type PromotableStorage interface {
On 2014/01/29 08:12:59, axw wrote:
> I'm not keen on this. I think a better way to test, rather than
further exposing
> the guts of this package, would be to use a
net/http/httputil.ReverseProxy.

Okay, I'll revert most of this and write something independant.

https://codereview.appspot.com/56560043/diff/20001/provider/common/state.go
File provider/common/state.go (right):

https://codereview.appspot.com/56560043/diff/20001/provider/common/state.go#newcode71
provider/common/state.go:71: func LoadStateFromURL(url string,
disableSSLHostnameVerification bool) (*BootstrapState, error) {
On 2014/01/29 08:12:59, axw wrote:
> This is fine for now, but I wonder if we shouldn't have a method of
obtaining an
> http.Client for an environment. We now have HTTP proxy settings and
SSL hostname
> verification as options. We could centralise all that and pass an
http.Client in
> here.

Yes, on trunk there should be some refactoring like that. There are a
couple of places that want a http.Client obeying thig setting and we may
want more.

https://codereview.appspot.com/56560043/

« Back to merge proposal