Code review comment for lp:~cbehrens/nova/servers-search

Revision history for this message
Chris Behrens (cbehrens) wrote :

lazy load: The fix is actually described in the thread with the bug, but: OS API accesses Instance['virtual_interfaces']['network'], but instance_get_all() does not joinedload('virtual_interfaces.network'). When the sqlalchemy session becomes detached, OS API cannot do the query for 'network' when it's accessed. My new DB API routine joins it. I didn't add it to any other queries, because the OS API may be the only place where it's needed.

164: Sure

217: Ok.

215: I'd originally broken it out because I split the caller into the 1.0/1.1 Controllers, so it had 2 references. I'll merge it back in, since that's not the case anymore.

392: I didn't use a mapping table because the flipping needs to happen in a certain order (since
i'm renaming name -> display_name, but instance_name to name. Doing those in reverse order would clobber the 'name' if both exist in the query). But I know how I can solve that.

419: Yeah, I did this because I'm not sure 'fixed_ip' should be used via OS API. One should use 'ip' instead. If it's for EC2 only, fixed_ip this way is a lot more efficient query since it looks up via FixedIps table directly with no joins to find the entry. Decisions like these are difficult, as I'm not entirely sure what's most desirable to the community...but generally there is some sort of reason to the method of my madness. :) I'm cool with changing it.

480: I feel this way, too.. I was just trying to match the original behavior in single zone. I guess if EC2 should really return an error in this case, the error return should happen in the ec2 controller if it gets an empty list back.

« Back to merge proposal