Merge lp:~ed-leafe/nova/lp785843 into lp:~hudson-openstack/nova/trunk

[Ed Leafe]

The code for getting an opaque reference to an instance assumed that there was a reference to an instance obj available when raising an exception. I changed this from raising an InstanceNotFound exception to a NotFound, as this is more appropriate for the failure, and doesn't require an instance ID.

[Rick Harris (rconradharris)]

lgtm. Thanks Ed.

review: Approve

[Josh Kearney (jk0)]

LGTM.

review: Approve

[Mark Washenberger (markwash)]

Looks like a good fix--and I can't find anywhere above this code that would appear to require specifically an InstanceNotFound exception.

Sure wish we could add unit tests for this stuff ;-)

review: Approve

[Vish Ishaya (vishvananda)]

It seems like the standard practice in the rest of the code base is to define a new type of NotFound exception for this and put the error message there. Not sure why that method wasn't used here...

On May 25, 2011, at 8:07 AM, Ed Leafe wrote:

> Ed Leafe has proposed merging lp:~ed-leafe/nova/lp785843 into lp:nova.
>
> Requested reviews:
> Nova Core (nova-core)
> Related bugs:
> Bug #785843 in OpenStack Compute (nova): "Traceback in xenapi/vmops.py masks real exception"
> https://bugs.launchpad.net/nova/+bug/785843
>
> For more details, see:
> https://code.launchpad.net/~ed-leafe/nova/lp785843/+merge/62321
>
> The code for getting an opaque reference to an instance assumed that there was a reference to an instance obj available when raising an exception. I changed this from raising an InstanceNotFound exception to a NotFound, as this is more appropriate for the failure, and doesn't require an instance ID.
> --
> https://code.launchpad.net/~ed-leafe/nova/lp785843/+merge/62321
> You are subscribed to branch lp:nova.
> === modified file 'nova/virt/xenapi/vmops.py'
> --- nova/virt/xenapi/vmops.py 2011-05-24 22:13:59 +0000
> +++ nova/virt/xenapi/vmops.py 2011-05-25 15:06:14 +0000
> @@ -253,7 +253,8 @@
> instance_name = instance_or_vm.name
> vm_ref = VMHelper.lookup(self._session, instance_name)
> if vm_ref is None:
> - raise exception.InstanceNotFound(instance_id=instance_obj.id)
> + raise exception.NotFound(_("No opaque_ref could be determined "
> + "for '%s'.") % instance_or_vm)
> return vm_ref
>
> def _acquire_bootlock(self, vm):
>

=== modified file 'nova/virt/xenapi/vmops.py'
--- nova/virt/xenapi/vmops.py	2011-05-24 22:13:59 +0000
+++ nova/virt/xenapi/vmops.py	2011-05-25 15:06:14 +0000
@@ -253,7 +253,8 @@
             instance_name = instance_or_vm.name
         vm_ref = VMHelper.lookup(self._session, instance_name)
         if vm_ref is None:
-            raise exception.InstanceNotFound(instance_id=instance_obj.id)
+            raise exception.NotFound(_("No opaque_ref could be determined "
+                    "for '%s'.") % instance_or_vm)
         return vm_ref
 
     def _acquire_bootlock(self, vm):