Simplestreams query for Azure.

Simplestreams query for Azure.

This adds to Azure the ability to query simplestreams for suitable OS images.
It's not hooked up into combined instance-type and image selection; that's for
a later branch.

Testing this was tough.  It's very hard to figure out why this isn't working
when it isn't — all you get is an empty list of matches.  I would have done
more unit-testing, but I'm exhausted.

There's also a little drive-by cleanup: the base test suite for Azure created
an Environ for tests to use.  This is bad practice, but luckily no tests were
making use of it.  I removed it before anyone gets it into their heads to make
use of the thing.

https://code.launchpad.net/~jtv/juju-core/az-simplestreams/+merge/177592

(do not edit description out of merge proposal)

[jtv.canonical, 2013-07-30 13:34:06]

Please take a look.

[rog, 2013-07-30 16:17:22]

LGTM with one thought below.

(sorry about the duplication - rietveld can't seem to show the comments for me
to delete)

https://codereview.appspot.com/12103043/diff/1/environs/azure/instancetype_te...
File environs/azure/instancetype_test.go (right):

https://codereview.appspot.com/12103043/diff/1/environs/azure/instancetype_te...
environs/azure/instancetype_test.go:233: func
prepareSimpleStreamsResponse(location, series, release, arch, json string)
func() {
another possibility might be to mock out simplestreams.Fetch as a function
pointer and check that it gets called with the right arguments.

that would seem to be at least no worse than second-guessing the
http requests issued by the simplestreams package.

https://codereview.appspot.com/12103043/diff/1/environs/azure/instancetype_te...
environs/azure/instancetype_test.go:233: func
prepareSimpleStreamsResponse(location, series, release, arch, json string)
func() {
another possibility might be to mock out simplestreams.Fetch as a function
pointer and check that it gets called with the right arguments.

that would seem to be at least no worse than second-guessing the
http requests issued by the simplestreams package.

[fwereade, 2013-07-31 02:23:48]

LGTM, thanks.

[jtv.canonical, 2013-07-31 03:58:14]

On 2013/07/30 16:17:22, rog wrote:

> (sorry about the duplication - rietveld can't seem to show the comments for me
> to delete)

I've seen that also, and strangely, after I entered an additional comment on the
same source line I got to see both the old and the new ones.

Frankly I don't think lbox and Rietveld are worth the bother for us.  Launchpad
is by no means perfect, but the failure rates and limitations of additional
moving parts add up, compared to using the company's standard process and
integrated tooling.  Case in point: Rietveld failed again while I was submitting
this reply.  It does that all the time.

That offsets the benefit of complementary features — inline reviewing being the
only one I'm aware of, and one that I personally of course am used to living
without.

The split in processes, and the NIH direction that originally caused it, have
also fueled discord in our engineering culture, hindering cross-team learning. 
Given the cost of mistakes from other teams that are being repeated, I would see
more value in fitting in with Launchpad like the rest of the company does.


>
https://codereview.appspot.com/12103043/diff/1/environs/azure/instancetype_te...
> environs/azure/instancetype_test.go:233: func
> prepareSimpleStreamsResponse(location, series, release, arch, json string)
> func() {
> another possibility might be to mock out simplestreams.Fetch as a function
> pointer and check that it gets called with the right arguments.
> 
> that would seem to be at least no worse than second-guessing the
> http requests issued by the simplestreams package.

Good point.  We did this in gwacl.  I do agree it's better for unit-testing, but
in this case of course I also needed an integration test.  In my next branch I
think I'll add the function pointer, because I'll be doing stuff where the
function can be patched out completely.  Maybe I can even make the
index-generation code reusable as a test helper, to cut down on inexplicable
hard-coded JSON in tests elsewhere.

There are some drawbacks to the function-pointer approach to monkey-patching. 
It adds an explicit layer of indirection and so increases cognitive load. 
Static typing increases maintenance friction for function signatures.  Making
the pointers persistent variables poses risks for concurrent testing, so they
belong in the nearest struct type, where injection may not be convenient — so
may require injecting entire factories, which works a lot better in a language
with optional keyword arguments.  Finally, lots of functions probably deserve to
be patched out, so before you know it you're manually simulating dynamic
dispatch.  I sometimes wish static languages could relax their rules in test
code, where performance doesn't matter so much...