Code review comment for lp:~cbehrens/nova/lp844160-build-works-with-zones

First off, I love that fact that you're keeping the unit tests as unit tests (and not integration tests) ... makes the review so much easier to follow.

I guess we really need update the docs shortly after this lands.

Regarding the precooked stuff, I wonder if we could just assume all results are raw and strip out any potentially offending data regardless? Just be a little more forgiving if they don't exist.

825 +from nova.rpc.common import RemoteError

import module not class

1697 + instances = self._schedule_run_instance(
I thought that method returned a tuple? Is this correct?

1950 + instance = self.create_instance_db_entry(context, request_spec)

create_instance_db_entry is a static method. Should either be classname qualifier or remove the @staticmethod decorator, not self.

2032 # Return instance as a list and make sure the caller doesn't
2033 + # cast to a compute node.

 ... not sure what the comment is trying to tell me.

2053 - # Returning None short-circuits the routing to Compute (since

... is this comment not appropriate anymore? I think some explanation of the None return is required (somewhere).

2215 + # Should only be None for tests?
2216 + if filename is not None:
Then this logic should be broken out into a separate function and stubbed in the test. Test case code shouldn't be in production code.

2256 + if isinstance(ret_val, tuple):
ah ha ... there it is. Can we unify these results to always be a tuple? Or I think we'd need a test for each condition (unless I missed something there)?

All in all ... great changes Chris! Nice to see that zone-boot and inheritance mess go away!