PTAL


https://codereview.appspot.com/80280043/diff/120001/cmd/juju/bootstrap_test.go
File cmd/juju/bootstrap_test.go (right):

https://codereview.appspot.com/80280043/diff/120001/cmd/juju/bootstrap_test.go#newcode123
cmd/juju/bootstrap_test.go:123: useVersion :=
strings.Replace(test.version, "%LTS%", config.LatestLtsSeries(), 1)
On 2014/04/03 13:34:15, fwereade wrote:
> not quite sure that config is the right package for these -- but I'm
not sure I
> can think of a better one. Unless you want to create a tiny,
hyper-focused,
> series package somewhere? I'm sure it'll grow excitingly as we deal
with other
> OSs...

LP: #1301999

https://codereview.appspot.com/80280043/diff/120001/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (right):

https://codereview.appspot.com/80280043/diff/120001/cmd/juju/upgradecharm.go#newcode209
cmd/juju/upgradecharm.go:209: repo, err :=
charm.InferRepository(newURL.Reference, c.RepoPath)
On 2014/04/03 13:34:15, fwereade wrote:
> We still need ctx.AbsPath, I think. Would you change the relevant test
to use a
> path relative to the working dir so we check this properly please?

Ah, must have botched this in a prior merge. Restored it.

https://codereview.appspot.com/80280043/diff/120001/environs/config/config.go
File environs/config/config.go (right):

https://codereview.appspot.com/80280043/diff/120001/environs/config/config.go#newcode106
environs/config/config.go:106:
On 2014/04/03 13:34:15, fwereade wrote:
> yeah, this feels a bit tacked-on here. Let's give it its own package
if we don't
> think of anywhere better.

LP: #1301999

https://codereview.appspot.com/80280043/diff/120001/environs/config/config.go#newcode456
environs/config/config.go:456: series := s.(string)
On 2014/04/03 13:34:15, fwereade wrote:
> this is guaranteed to be a string if it's got this far, right?

Non-string values that make their way in here will be ignored with a
warning.

https://codereview.appspot.com/80280043/diff/120001/provider/common/bootstrap.go
File provider/common/bootstrap.go (right):

https://codereview.appspot.com/80280043/diff/120001/provider/common/bootstrap.go#newcode45
provider/common/bootstrap.go:45: selectedTools, err :=
EnsureBootstrapTools(ctx, env, config.PreferredSeries(env.Config()),
cons.Arch)
On 2014/04/03 13:34:15, fwereade wrote:
> quibble quibble, we should probably allow bootstrap on any series we
can find
> tools for. Out of scope today, but maybe worth a bug?

LP: #1302005

https://codereview.appspot.com/80280043/diff/120001/provider/ec2/live_test.go
File provider/ec2/live_test.go (right):

https://codereview.appspot.com/80280043/diff/120001/provider/ec2/live_test.go#newcode104
provider/ec2/live_test.go:104: Series: coretesting.FakeDefaultSeries,
On 2014/04/03 13:34:15, fwereade wrote:
> wondering if this should be LatestLts? I presume you have a good
reason it
> isn't, but it's not immediately clear ;)

It's to separate what is used for default-series: in tests from the
preferred series selection. This allows us to test scenarios against
different configured series vs. what will be preferred.

Most of what's affected atm is the tools setup & bootstrapping for some
tests. There are still some areas lurking where its hard-coded to
precise.

I've opened LP: #1302015 to address this.

https://codereview.appspot.com/80280043/diff/120001/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/80280043/diff/120001/state/api/client.go#newcode613
state/api/client.go:613: func (c *Client) ResolveCharms(refs
...charm.Reference) ([]params.ResolveCharmResult, error) {
On 2014/04/03 13:34:15, fwereade wrote:
> I think this client-side method might be cleaner as non-bulk -- I
don't really
> care about bulk calls and compatibility for state/api, just for
state/apiserver.


Done.

https://codereview.appspot.com/80280043/diff/120001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/80280043/diff/120001/state/api/params/params.go#newcode335
state/api/params/params.go:335: References []charm.Reference
On 2014/04/03 13:34:15, fwereade wrote:
> remind me, does this jsonify to a string?

It does now.

https://codereview.appspot.com/80280043/diff/120001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/80280043/diff/120001/state/apiserver/client/client.go#newcode576
state/apiserver/client/client.go:576: prefSeries =
config.PreferredSeries(conf)
On 2014/04/03 13:34:15, fwereade wrote:
> is it rational to move all this block...

Done.

https://codereview.appspot.com/80280043/diff/120001/state/apiserver/client/client.go#newcode595
state/apiserver/client/client.go:595: p.Series = prefSeries
On 2014/04/03 13:34:15, fwereade wrote:
> ...into here? and not hit the db for config unless it's really needed?

Done.

https://codereview.appspot.com/80280043/diff/120001/state/apiserver/client/client.go#newcode985
state/apiserver/client/client.go:985: return store.Resolve(ref)
On 2014/04/03 13:34:15, fwereade wrote:
> I suspect it would be nicer to move this stuff into ResolveCharm to
avoid the
> repeated db hits

Done.

https://codereview.appspot.com/80280043/