Remove instances of the "diaper pattern"

Anywhere "except:" occurs, I tried to replace it with an explicit except on
known error types. If none were known, Except was used.

In the process I've been able to unearth a few evasive bugs and clean up some
adjacent code.

nova/api/direct.py

@@ -48,7 +48,6 @@ import nova.api.openstack.wsgi
 # Global storage for registering modules.
 ROUTES = {}
 
-
 def register_service(path, handle):
     """Register a service handle at a given path.
 
@@ -296,8 +295,8 @@ class ServiceWrapper(object):
               'application/json': nova.api.openstack.wsgi.JSONDictSerializer(),
             }[content_type]
             return serializer.serialize(result)
-        except:
+        except Exception, e:
-            raise exception.Error("returned non-serializable type: %s"
+            raise exception.Error(_("Returned non-serializable type: %s")
                                   % result)
 
 

nova/api/ec2/__init__.py

@@ -147,7 +147,7 @@ class Authenticate(wsgi.Middleware):
         try:
             signature = req.params['Signature']
             access = req.params['AWSAccessKeyId']
-        except:
+        except KeyError, e:
             raise webob.exc.HTTPBadRequest()
 
         # Make a copy of args for authentication and signature verification.
@@ -211,7 +211,7 @@ class Requestify(wsgi.Middleware):
             for non_arg in non_args:
                 # Remove, but raise KeyError if omitted
                 args.pop(non_arg)
-        except:
+        except KeyError, e:
             raise webob.exc.HTTPBadRequest()
 
         LOG.debug(_('action: %s'), action)

nova/api/ec2/apirequest.py

@@ -104,7 +104,7 @@ class APIRequest(object):
             for key in data.keys():
                 val = data[key]
                 el.appendChild(self._render_data(xml, key, val))
-        except:
+        except Exception:
             LOG.debug(data)
             raise
 

nova/api/openstack/common.py

@@ -16,7 +16,7 @@
 #    under the License.
 
 import re
-from urlparse import urlparse
+import urlparse
 from xml.dom import minidom
 
 import webob
@@ -137,8 +137,8 @@ def get_id_from_href(href):
     if re.match(r'\d+$', str(href)):
         return int(href)
     try:
-        return int(urlparse(href).path.split('/')[-1])
+        return int(urlparse.urlsplit(href).path.split('/')[-1])
-    except:
+    except ValueError, e:
         LOG.debug(_("Error extracting id from href: %s") % href)
         raise ValueError(_('could not parse id from href'))
 
@@ -153,22 +153,17 @@ def remove_version_from_href(href):
     Returns: 'http://www.nova.com'
 
     """
-    try:
+    parsed_url = urlparse.urlsplit(href)
-        #removes the first instance that matches /v#.#/
+    new_path = re.sub(r'^/v[0-9]+\.[0-9]+(/|$)', r'\1', parsed_url.path, count=1)
-        new_href = re.sub(r'[/][v][0-9]+\.[0-9]+[/]', '/', href, count=1)
 
-        #if no version was found, try finding /v#.# at the end of the string
+    if new_path == parsed_url.path:
-        if new_href == href:
+        msg = _('href %s does not contain version') % href
-            new_href = re.sub(r'[/][v][0-9]+\.[0-9]+$', '', href, count=1)
+        LOG.debug(msg)
-    except:
-        LOG.debug(_("Error removing version from href: %s") % href)
-        msg = _('could not parse version from href')
         raise ValueError(msg)
 
-    if new_href == href:
+    parsed_url = list(parsed_url)
-        msg = _('href does not contain version')
+    parsed_url[2] = new_path
-        raise ValueError(msg)
+    return urlparse.urlunsplit(parsed_url)
-    return new_href
 
 
 def get_version_from_href(href):

nova/api/openstack/servers.py

@@ -290,7 +290,7 @@ class Controller(object):
         context = req.environ['nova.context']
         try:
             self.compute_api.lock(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::lock %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -306,7 +306,7 @@ class Controller(object):
         context = req.environ['nova.context']
         try:
             self.compute_api.unlock(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::unlock %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -321,7 +321,7 @@ class Controller(object):
         context = req.environ['nova.context']
         try:
             self.compute_api.get_lock(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::get_lock %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -336,7 +336,7 @@ class Controller(object):
         context = req.environ['nova.context']
         try:
             self.compute_api.reset_network(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::reset_network %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -351,7 +351,7 @@ class Controller(object):
         context = req.environ['nova.context']
         try:
             self.compute_api.inject_network_info(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::inject_network_info %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -363,7 +363,7 @@ class Controller(object):
         ctxt = req.environ['nova.context']
         try:
             self.compute_api.pause(ctxt, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::pause %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -375,7 +375,7 @@ class Controller(object):
         ctxt = req.environ['nova.context']
         try:
             self.compute_api.unpause(ctxt, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::unpause %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -387,7 +387,7 @@ class Controller(object):
         context = req.environ['nova.context']
         try:
             self.compute_api.suspend(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("compute.api::suspend %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -399,7 +399,7 @@ class Controller(object):
         context = req.environ['nova.context']
         try:
             self.compute_api.resume(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("compute.api::resume %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -420,7 +420,7 @@ class Controller(object):
         context = req.environ["nova.context"]
         try:
             self.compute_api.rescue(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("compute.api::rescue %s"), readable)
             raise exc.HTTPUnprocessableEntity()
@@ -432,7 +432,7 @@ class Controller(object):
         context = req.environ["nova.context"]
         try:
             self.compute_api.unrescue(context, id)
-        except:
+        except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("compute.api::unrescue %s"), readable)
             raise exc.HTTPUnprocessableEntity()

nova/cloudpipe/pipelib.py

@@ -141,15 +141,12 @@ class CloudPipe(object):
         try:
             result = cloud._gen_key(context, context.user_id, key_name)
             private_key = result['private_key']
-            try:
+            key_dir = os.path.join(FLAGS.keys_path, context.user_id)
-                key_dir = os.path.join(FLAGS.keys_path, context.user_id)
+            if not os.path.exists(key_dir):
-                if not os.path.exists(key_dir):
+                os.makedirs(key_dir)
-                    os.makedirs(key_dir)
+            key_path = os.path.join(key_dir, '%s.pem' % key_name)
-                key_path = os.path.join(key_dir, '%s.pem' % key_name)
+            with open(key_path, 'w') as f:
-                with open(key_path, 'w') as f:
+                f.write(private_key)
-                    f.write(private_key)
+        except exception.Duplicate, os.error, IOError:
-            except:
-                pass
-        except exception.Duplicate:
             pass
         return key_name

nova/db/sqlalchemy/api.py

@@ -3178,8 +3178,9 @@ def instance_metadata_delete_all(context, instance_id):
 
 @require_context
 @require_instance_exists
-def instance_metadata_get_item(context, instance_id, key):
+def instance_metadata_get_item(context, instance_id, key, session=None):
-    session = get_session()
+    if not session:
+        session = get_session()
 
     meta_result = session.query(models.InstanceMetadata).\
                     filter_by(instance_id=instance_id).\
@@ -3205,7 +3206,7 @@ def instance_metadata_update_or_create(context, instance_id, metadata):
         try:
             meta_ref = instance_metadata_get_item(context, instance_id, key,
                                                         session)
-        except:
+        except exception.InstanceMetadataNotFound, e:
             meta_ref = models.InstanceMetadata()
         meta_ref.update({"key": key, "value": value,
                             "instance_id": instance_id,
@@ -3300,8 +3301,9 @@ def instance_type_extra_specs_delete(context, instance_type_id, key):
 
 
 @require_context
-def instance_type_extra_specs_get_item(context, instance_type_id, key):
+def instance_type_extra_specs_get_item(context, instance_type_id, key, session=None):
-    session = get_session()
+    if not session:
+        session = get_session()
 
     spec_result = session.query(models.InstanceTypeExtraSpecs).\
                     filter_by(instance_type_id=instance_type_id).\
@@ -3327,7 +3329,7 @@ def instance_type_extra_specs_update_or_create(context, instance_type_id,
                                                           instance_type_id,
                                                           key,
                                                           session)
-        except:
+        except exception.InstanceTypeExtraSpecsNotFound, e:
             spec_ref = models.InstanceTypeExtraSpecs()
         spec_ref.update({"key": key, "value": value,
                          "instance_type_id": instance_type_id,

nova/image/__init__.py

@@ -35,6 +35,7 @@ def _parse_image_ref(image_href):
 
     :param image_href: href of an image
     :returns: a tuple of the form (image_id, host, port)
+    :raises ValueError
 
     """
     o = urlparse(image_href)
@@ -72,7 +73,7 @@ def get_glance_client(image_href):
 
     try:
         (image_id, host, port) = _parse_image_ref(image_href)
-    except:
+    except ValueError:
         raise exception.InvalidImageRef(image_href=image_href)
     glance_client = GlanceClient(host, port)
     return (glance_client, image_id)

nova/utils.py

@@ -126,6 +126,22 @@ def fetchfile(url, target):
 
 
 def execute(*cmd, **kwargs):
+    """
+    Helper method to execute command with optional retry.
+
+    :cmd                Passed to subprocess.Popen.
+    :process_input      Send to opened process.
+    :addl_env           Added to the processes env.
+    :check_exit_code    Defaults to 0. Raise exception.ProcessExecutionError
+                        unless program exits with this code.
+    :delay_on_retry     True | False. Defaults to True. If set to True, wait a
+                        short amount of time before retrying.
+    :attempts           How many times to retry cmd.
+
+    :raises exception.Error on receiving unknown arguments
+    :raises exception.ProcessExecutionError
+    """
+
     process_input = kwargs.pop('process_input', None)
     addl_env = kwargs.pop('addl_env', None)
     check_exit_code = kwargs.pop('check_exit_code', 0)
@@ -790,7 +806,7 @@ def parse_server_string(server_str):
         (address, port) = server_str.split(':')
         return (address, port)
 
-    except:
+    except Exception:
         LOG.debug(_('Invalid server_string: %s' % server_str))
         return ('', '')
 

nova/virt/libvirt/connection.py

@@ -370,7 +370,7 @@ class LibvirtConnection(driver.ComputeDriver):
         """Returns the xml for the disk mounted at device"""
         try:
             doc = libxml2.parseDoc(xml)
-        except:
+        except Exception:
             return None
         ctx = doc.xpathNewContext()
         try:
@@ -1103,7 +1103,7 @@ class LibvirtConnection(driver.ComputeDriver):
 
         try:
             doc = libxml2.parseDoc(xml)
-        except:
+        except Exception:
             return []
 
         ctx = doc.xpathNewContext()
@@ -1144,7 +1144,7 @@ class LibvirtConnection(driver.ComputeDriver):
 
         try:
             doc = libxml2.parseDoc(xml)
-        except:
+        except Exception:
             return []
 
         ctx = doc.xpathNewContext()

nova/virt/libvirt/vif.py

@@ -25,6 +25,7 @@ from nova.network import linux_net
 from nova.virt.libvirt import netutils
 from nova import utils
 from nova.virt.vif import VIFDriver
+from nova import exception
 
 LOG = logging.getLogger('nova.virt.libvirt.vif')
 
@@ -128,7 +129,7 @@ class LibvirtOpenVswitchDriver(VIFDriver):
             utils.execute('sudo', 'ovs-vsctl', 'del-port',
                           network['bridge'], dev)
             utils.execute('sudo', 'ip', 'link', 'delete', dev)
-        except:
+        except exception.ProcessExecutionError, e:
             LOG.warning(_("Failed while unplugging vif of instance '%s'"),
                         instance['name'])
-            raise
+            raise e

nova/virt/xenapi_conn.py

@@ -441,7 +441,7 @@ class XenAPISession(object):
                 params = None
                 try:
                     params = eval(exc.details[3])
-                except:
+                except Exception:
                     raise exc
                 raise self.XenAPI.Failure(params)
             else:

nova/vnc/proxy.py

@@ -60,7 +60,7 @@ class WebsocketVNCProxy(object):
                     break
                 d = base64.b64encode(d)
                 dest.send(d)
-        except:
+        except Exception:
             source.close()
             dest.close()
 
@@ -72,7 +72,7 @@ class WebsocketVNCProxy(object):
                     break
                 d = base64.b64decode(d)
                 dest.sendall(d)
-        except:
+        except Exception:
             source.close()
             dest.close()