Code review comment for lp:~wallyworld/juju-core/fix-instance-type-matching

LGTM, but an explicit tie-breaker would be really nice.

https://codereview.appspot.com/58950043/diff/40001/environs/instances/instancetype_test.go
File environs/instances/instancetype_test.go (right):

https://codereview.appspot.com/58950043/diff/40001/environs/instances/instancetype_test.go#newcode155
environs/instances/instancetype_test.go:155: about: "small mem
specified, use that even though less than needed for mongodb",
On 2014/02/11 09:59:57, dimitern wrote:
> On 2014/02/11 00:39:04, wallyworld wrote:
> > On 2014/02/10 08:27:13, dimitern wrote:
> > > I think if the user overrode our minimum memory heuristic, we
should at
> least
> > > print out a warning, saying bootstrap might fail due to
insufficient memory
> > for
> > > mongo.
> >
> > Not sure about that. The user could also specify poorly speced
systems for
> > charms and we don't emit anything there. This behaviour, allowing
the user to
> > specify small memory, is not new - the tests are just being
improved. We have
> > never in the past emitted a warning AFAIK.

> It's true we never said anything, but it's the UX improvement an
important goal?
> An overly conservative user might pick 256M so it's cheap, but then
will get
> frustrated why he can't bootstrap (perhaps without a nice error in the
log).

We *might* want to do this on a state server (although really we should
just take what we *know* we need in that case) but this isn't the right
layer IMO. They asked explicitly, and they got it ;).

https://codereview.appspot.com/58950043/diff/60001/environs/instances/instancetype.go
File environs/instances/instancetype.go (right):

https://codereview.appspot.com/58950043/diff/60001/environs/instances/instancetype.go#newcode99
environs/instances/instancetype.go:99: // - if no matches and no mem
constraint specified, try again and return any matching instance
"any": would be nice to tie-break equal-greatest-memory machines by cost

https://codereview.appspot.com/58950043/