Code review comment for lp://staging/~wallyworld/juju-core/image-metadata-defaults

Revision history for this message
William Reade (fwereade) wrote :

Need to go to lunch, but here's one preliminary comment that I think is
important

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

https://codereview.appspot.com/9303051/diff/1/environs/instances/instancetype.go#newcode102
environs/instances/instancetype.go:102: // No matching instance types
were found, so the fallback is to:
If no matching instance types were found, we should return an error.
Please don't preserve this incorrect behaviour.

I think you'd find life a *lot* easier if you stripped this function
down to one that filters a list of instance types by constraints *only*.
This will make it much easier to write the prefer-a-gig code, I think
(filter all for base constraints, if mem wasn't set filter those by mem
as well and replace if it doesn't mess with defaults) and do appropriate
jiggery-pokery to pick a sensible ordering.

https://codereview.appspot.com/9303051/

« Back to merge proposal