Merge lp:~vishvananda/nova/lp706072 into lp:~hudson-openstack/nova/trunk

[Vish Ishaya]

Wraps the NotFound exception at the api layer to print the proper instance id. Does the same for volume. Note that euca-describe-volumes doesn't pass in volume ids properly, so you will get no error messages on euca-describe-volumes with improper ids. We may also need to wrap a few other calls as well.

[Jay Pipes (jaypipes)]

Hi!

Code looks good! Need to i18n those strings in the exceptions, though! :)

-jay

review: Needs Fixing

[Vish Ishaya (vishvananda)]

DOH! Fixed

[lp:~vishvananda/nova/lp706072]

594. By Vish Ishaya on 2011-01-22

i18n!

[Devin Carlen (devcamcar)]

lgtm

review: Approve

[Rick Harris (rconradharris)]

lgtm

review: Approve

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2011-01-20 20:04:57 +0000
+++ nova/api/ec2/cloud.py	2011-01-22 00:05:24 +0000
@@ -529,11 +529,18 @@
 
     def describe_volumes(self, context, volume_id=None, **kwargs):
         if volume_id:
-            volume_id = [ec2_id_to_id(x) for x in volume_id]
-        volumes = self.volume_api.get_all(context)
-        # NOTE(vish): volume_id is an optional list of volume ids to filter by.
-        volumes = [self._format_volume(context, v) for v in volumes
-                   if volume_id is None or v['id'] in volume_id]
+            volumes = []
+            for ec2_id in volume_id:
+                internal_id = ec2_id_to_id(ec2_id)
+                try:
+                    volume = self.volume_api.get(context, internal_id)
+                    volumes.append(volume)
+                except exception.NotFound:
+                    raise exception.NotFound(_("Volume %s not found")
+                                             % ec2_id)
+        else:
+            volumes = self.volume_api.get_all(context)
+        volumes = [self._format_volume(context, v) for v in volumes]
         return {'volumeSet': volumes}
 
     def _format_volume(self, context, volume):
@@ -657,8 +664,15 @@
         reservations = {}
         # NOTE(vish): instance_id is an optional list of ids to filter by
         if instance_id:
-            instance_id = [ec2_id_to_id(x) for x in instance_id]
-            instances = [self.compute_api.get(context, x) for x in instance_id]
+            instances = []
+            for ec2_id in instance_id:
+                internal_id = ec2_id_to_id(ec2_id)
+                try:
+                    instance = self.compute_api.get(context, internal_id)
+                    instances.append(instance)
+                except exception.NotFound:
+                    raise exception.NotFound(_("Instance %s not found")
+                                             % ec2_id)
         else:
             instances = self.compute_api.get_all(context, **kwargs)
         for instance in instances:

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-01-20 01:34:54 +0000
+++ nova/db/sqlalchemy/api.py	2011-01-22 00:05:24 +0000
@@ -1395,11 +1395,13 @@
 
     if is_admin_context(context):
         result = session.query(models.Volume).\
+                         options(joinedload('instance')).\
                          filter_by(id=volume_id).\
                          filter_by(deleted=can_read_deleted(context)).\
                          first()
     elif is_user_context(context):
         result = session.query(models.Volume).\
+                         options(joinedload('instance')).\
                          filter_by(project_id=context.project_id).\
                          filter_by(id=volume_id).\
                          filter_by(deleted=False).\