Code review comment for lp:~hazmat/pyjuju/ostack-constraints

Generic whine about mixing imports rearrangement into a code change that
needs review, could easily have been a separate branch to make life
easier.

https://codereview.appspot.com/6421053/diff/1/juju/providers/common/instance_type.py
File juju/providers/common/instance_type.py (right):

https://codereview.appspot.com/6421053/diff/1/juju/providers/common/instance_type.py#newcode25
juju/providers/common/instance_type.py:25: def _solve(self,
constraints):
Not that it really matters given the small number of flavors generally,
but all this faffing around with filters seems complicated given these
constraints have order, and sort/bisect would work just as well.

https://codereview.appspot.com/6421053/diff/1/juju/providers/common/instance_type.py#newcode35
juju/providers/common/instance_type.py:35: key=lambda x:
self._instance_types[x].cpu)[0]
Instead just do `possible_types.sort(); return possible_types[0]` which
would give a deterministic result for instances with same cpu but
different mem as well?

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/client.py
File juju/providers/openstack/client.py (right):

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/client.py#newcode321
juju/providers/openstack/client.py:321: def list_flavors_details(self):
Rename to list_flavor_detail so it's the same form as the others and the
url.

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/launch.py
File juju/providers/openstack/launch.py (right):

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/launch.py#newcode84
juju/providers/openstack/launch.py:84: flavors = yield
self._provider.nova.list_flavors_details()
Rename list_flavors_details -> list_flavors_detail.

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/launch.py#newcode120
juju/providers/openstack/launch.py:120: def _solve_flavor(self,
flavor_name, flavors):
Private helper isn't related to launch and doesn't use self. I'd make it
a function rather than a method if we're not going down the route of an
interface on the provider.

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/provider.py
File juju/providers/openstack/provider.py (right):

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/provider.py#newcode67
juju/providers/openstack/provider.py:67: # Fetch provider defined
instance types (just names)
I'd prefer if this created a provider instance shared store of flavor
information, that launch then accessed to resolve its constraints. Would
mean one list_flavor_detail call rather than one or two per launch, and
would also tie flavor business into a single class.

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/tests/test_launch.py
File juju/providers/openstack/tests/test_launch.py (right):

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/tests/test_launch.py#newcode30
juju/providers/openstack/tests/test_launch.py:30:
self.nova.list_flavors_details()
Rename list_flavors_details -> list_flavors_detail.

https://codereview.appspot.com/6421053/