Code review comment for lp:~axwalk/juju-core/lp1233924-simplestreams-getmetadata-fallback

Revision history for this message
Andrew Wilkins (axwalk) wrote :

On 2013/10/02 05:07:52, axw1 wrote:

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go
> File environs/simplestreams/simplestreams_test.go (right):

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode300
> environs/simplestreams/simplestreams_test.go:300: Series:
[]string{"precise"},
> // never match
> On 2013/10/02 04:02:00, thumper wrote:
> > What is the never match? precise or arm? Also, perhaps better to
give a
> region
> > that doesn't exist? Someone may add arm at some stage. Would that
work?

> precise, which isn't in the daily stream I added to testing. I can't
use a
> non-existent region, as that causes GetMetadata to error, which we
don't want.

> The series are validated, but architectures aren't. I'll put in an
invalid arch.

https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode326
> environs/simplestreams/simplestreams_test.go:326: messages =
append(messages,
> messages...)
> On 2013/10/02 04:02:00, thumper wrote:
> > I agree with our IRC chat, ideally we would have a simple mock
datasource.
> What
> > about...
> >
> > type countingSource struct {
> > simplestreams.DataSource
> > count int
> > }
> >
> > func (s *countingSource) URL(path string) (string, error) {
> > s.count++
> > return s.DataSource.URL(path)
> > }
> >
> > Then have sources defined by:
> >
> > first :=
> >
&countingSource{DataSource:simplestreams.NewURLDataSource("test:/daily",
> > simplestreams.VerifySSLHostnames)}
> > second :=
> >
&countingSource{DataSource:simplestreams.NewURLDataSource("test:/daily",
> > simplestreams.VerifySSLHostnames)}
> > sources := []simplestream.DataSource{first, second}
> >
> > // do stuff
> >
> > c.Check(first.count, gc.Equals, 1)
> > c.Check(second.count, gc.Equals, 1)
> >

> Done, tho I'm reusing a single countingSource.

So, after discussing with jam, GetMetadata *might* be how it is on
purpose. You might want to populate metadata into your private cloud,
and then expect only to be able to create images that match your private
metadata.

I'll leave this for now, until it's decided how it should be working.

https://codereview.appspot.com/14218044/

« Back to merge proposal