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

LGTM +1

Just a couple of nitpicks. Otherwise this looks good and brings some of
the conventions more in line with the coding std.

I glanced at the Nova docs, it didn't look like they supported 'arch' on
a quick read, I did find that surprising.

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#newcode16
juju/providers/common/instance_type.py:16: instance_type =
constraints["instance-type"]
.get("instance-type") might be more defensive as I know you can handle
the None, but maybe not a KeyError.

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#newcode83
juju/providers/openstack/launch.py:83: "default-instance-type is
deprecated, use cli --constraints")
With no users of the provider yet (or are there) you could remove the
option from the config and skip even the warning.

https://codereview.appspot.com/6421053/diff/1/juju/providers/openstack/launch.py#newcode125
juju/providers/openstack/launch.py:125: '686', f['vcpus'], f['ram'])
I see that the underlying impl won't search 'arch' but using a fake
value here that looks real might be an issue down the line. Maybe
'ignored-arch' or 'unused' is better?

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#newcode75
juju/providers/openstack/tests/test_launch.py:75: return
provider.launch("1", constraints=["cpu=2", "mem=3G"])
Isn't the return value ignored here, unless I'm missing something the
'return' is undesirable on any of these tests.

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