environs/imagemetadat: tweak debug messages

environs/imagemetadat: tweak debug messages

We were generating messages when we failed to read an index file, but
didn't have one when we succeeded. Which meant it looks like things
were failing when really we were just moving to a fallback.

Also, we had a formatting error "%q/%q" causes each part of the URL to
get its own " characters. We only want the whole string quoted.

I extracted out the base+path code into urlJoin, because I didn't see
something obvious elswhere in the code, I just have it as a local
function. We really should put this as a general helper, though.

https://code.launchpad.net/~jameinel/juju-core/simplestreams-debugging/+merge/180081

(do not edit description out of merge proposal)

[jameinel, 2013-08-14 10:03:46]

Please take a look.

[gz, 2013-08-14 16:41:46]

LGTM.

https://codereview.appspot.com/12834045/diff/1/environs/imagemetadata/simples...
File environs/imagemetadata/simplestreams.go (right):

https://codereview.appspot.com/12834045/diff/1/environs/imagemetadata/simples...
environs/imagemetadata/simplestreams.go:306: 
Why is the log line not using indexURL as well?

https://codereview.appspot.com/12834045/diff/1/environs/imagemetadata/simples...
environs/imagemetadata/simplestreams.go:318: 
There's really not a better place for this live? I guess it's not sufficent for
the general case, which is much more complex.

As such, I'd name this something other than urlJoin ideally. Or at least a
comment saying it doesn't handle queries or fragments.

[natefinch, 2013-08-14 18:58:30]

+1 to mgz's comments. Otherwise LGTM

[jameinel, 2013-08-18 06:52:36]

Please take a look.

https://codereview.appspot.com/12834045/diff/1/environs/imagemetadata/simples...
File environs/imagemetadata/simplestreams.go (right):

https://codereview.appspot.com/12834045/diff/1/environs/imagemetadata/simples...
environs/imagemetadata/simplestreams.go:306: 
On 2013/08/14 16:41:46, gz wrote:
> Why is the log line not using indexURL as well?

It was meant to, I though I got it, but clearly I missed it. Thanks for the
catch.

https://codereview.appspot.com/12834045/diff/1/environs/imagemetadata/simples...
environs/imagemetadata/simplestreams.go:318: 
On 2013/08/14 16:41:46, gz wrote:
> There's really not a better place for this live? I guess it's not sufficent
for
> the general case, which is much more complex.
> 
> As such, I'd name this something other than urlJoin ideally. Or at least a
> comment saying it doesn't handle queries or fragments.

I started to do that, but I realize for what this code actually is,
'path.Join()' works. It did require a change to one bit.

I'm slightly concerned about what path.Join will do if relpath starts with a /
(path.Join('/foo', '/bar') could easily be '/bar' if it treats them as local
paths.)

But that isn't what I get testing it here:
http://play.golang.org/p/THDz2jr21r

[fwereade, 2013-08-21 08:15:56]

LGTM