Code review comment for lp:~gz/pyjuju/openstack_provider

Thanks for the comments! Have replied to some below, will act on the
others.

https://codereview.appspot.com/6312050/diff/1/juju/errors.py
File juju/errors.py (right):

https://codereview.appspot.com/6312050/diff/1/juju/errors.py#newcode162
juju/errors.py:162: ", ".join(map(str, self.instance_ids)))
Instance ids are normally UUIDs and stored as strings, but Nova
currently also allows the integer form of the ec2 id. This shouldn't
come up in production, but is exercised in tests as writing out UUIDs is
annoying.

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

https://codereview.appspot.com/6312050/diff/1/juju/providers/openstack/client.py#newcode309
juju/providers/openstack/client.py:309: server["accessIPv4"] = ip
Serves no purpose now, was added when I was experimenting with ways to
solve the problem of juju needing a public ip.

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

https://codereview.appspot.com/6312050/diff/1/juju/providers/openstack/files.py#newcode78
juju/providers/openstack/files.py:78: if response.code != 201:
Right, the swift backend should probably raise exceptions then this
block could reraise the original one.

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

https://codereview.appspot.com/6312050/diff/1/juju/providers/openstack/launch.py#newcode61
juju/providers/openstack/launch.py:61: if self._master:
The metadata service only exposes the EC2 form of the server id, but the
rest of the code deals with the UUID form. Sticking the UUID in file
storage stops juju getting confused with two different ids for the same
server. Trick should no longer be required with the Folsom release, as
the metadata service will be adding some OpenStack values.

https://codereview.appspot.com/6312050/diff/1/juju/providers/openstack/launch.py#newcode89
juju/providers/openstack/launch.py:89: yield filestorage.put(id_name,
StringIO(server['id']))
Not had any problems in testing, the conditions should heavily favour
the correct outcome. Sticking a small string in the object store should
beat the server boot process even giving it a head start.

https://codereview.appspot.com/6312050/diff/1/juju/providers/openstack/launch.py#newcode99
juju/providers/openstack/launch.py:99: reactor.callLater(5,
deferred.callback, None)
Doesn't need to finish booting, but doesn't work till the network setup
has been done, which is not synchronous on creating the server.

This is one the reasons I'd really like the public ip assignment to move
to the port handling code, and juju to not expect a public address for
all units.

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

https://codereview.appspot.com/6312050/diff/1/juju/providers/openstack/provider.py#newcode198
juju/providers/openstack/provider.py:198: return defer.gatherResults(
I saw that helper but it wasn't obvious what it was adding. I think the
correct behaviour here requires using DeferredList directly and
examining exceptions anyway.

https://codereview.appspot.com/6312050/