]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Remove usage of locals() for formatting from cinder.api.*
authorHaomai Wang <haomai@unitedstack.com>
Fri, 14 Jun 2013 07:11:29 +0000 (15:11 +0800)
committerHaomai Wang <haomai@unitedstack.com>
Fri, 14 Jun 2013 11:02:47 +0000 (19:02 +0800)
Using of locals() for formatting string is a nasty thing because:
1) It is not so clear as using explicit dicts
2) It could produce hidden errors during refactoring
3) Changing name of variable causes change in message
4) Creating a lot of unused variables

Fix bug 1171936

Change-Id: Id6a900899db328be067b8139a49c12ce802dd415

cinder/api/contrib/backups.py
cinder/api/contrib/hosts.py
cinder/api/extensions.py
cinder/api/openstack/__init__.py
cinder/api/v1/volumes.py
cinder/api/xmlutil.py

index 81c8953724127ed4818687644a8436503976b980..ac4bd3552c4dabe320dc77aaf4e45f5c8993b03a 100644 (file)
@@ -199,7 +199,9 @@ class BackupsController(wsgi.Controller):
         description = backup.get('description', None)
 
         LOG.audit(_("Creating backup of volume %(volume_id)s in container"
-                    " %(container)s"), locals(), context=context)
+                    " %(container)s"),
+                  {'volume_id': volume_id, 'container': container},
+                  context=context)
 
         try:
             new_backup = self.backup_api.create(context, name, description,
@@ -217,8 +219,8 @@ class BackupsController(wsgi.Controller):
     @wsgi.deserializers(xml=RestoreDeserializer)
     def restore(self, req, id, body):
         """Restore an existing backup to a volume."""
-        backup_id = id
-        LOG.debug(_('Restoring backup %(backup_id)s (%(body)s)') % locals())
+        LOG.debug(_('Restoring backup %(backup_id)s (%(body)s)'),
+                  {'backup_id': id, 'body': body})
         if not self.is_valid_body(body, 'restore'):
             raise exc.HTTPBadRequest()
 
@@ -232,11 +234,12 @@ class BackupsController(wsgi.Controller):
         volume_id = restore.get('volume_id', None)
 
         LOG.audit(_("Restoring backup %(backup_id)s to volume %(volume_id)s"),
-                  locals(), context=context)
+                  {'backup_id': id, 'volume_id': volume_id},
+                  context=context)
 
         try:
             new_restore = self.backup_api.restore(context,
-                                                  backup_id=backup_id,
+                                                  backup_id=id,
                                                   volume_id=volume_id)
         except exception.InvalidInput as error:
             raise exc.HTTPBadRequest(explanation=unicode(error))
index 1a853f3399018d77275e49165475d2e249da9a4c..aedc8db6c115c716079f45069bfd2b4b7a9cb875 100644 (file)
@@ -177,7 +177,8 @@ class HostController(object):
         """Sets the specified host's ability to accept new volumes."""
         context = req.environ['cinder.context']
         state = "enabled" if enabled else "disabled"
-        LOG.audit(_("Setting host %(host)s to %(state)s.") % locals())
+        LOG.audit(_("Setting host %(host)s to %(state)s."),
+                  {'host': host, 'state': state})
         result = self.api.set_host_enabled(context,
                                            host=host,
                                            enabled=enabled)
index 744adfcbd53e772a8af6c4484e0b917a4bf33d5d..da6042b65ffb57d423d9f75510170e164766f8a1 100644 (file)
@@ -288,7 +288,8 @@ class ExtensionManager(object):
                 self.load_extension(ext_factory)
             except Exception as exc:
                 LOG.warn(_('Failed to load extension %(ext_factory)s: '
-                           '%(exc)s') % locals())
+                           '%(exc)s'),
+                         {'ext_factory': ext_factory, 'exc': exc})
 
 
 class ControllerExtension(object):
@@ -356,7 +357,8 @@ def load_standard_extensions(ext_mgr, logger, path, package, ext_list=None):
                 ext_mgr.load_extension(classpath)
             except Exception as exc:
                 logger.warn(_('Failed to load extension %(classpath)s: '
-                              '%(exc)s') % locals())
+                              '%(exc)s'),
+                            {'classpath': classpath, 'exc': exc})
 
         # Now, let's consider any subdirectories we may have...
         subdirs = []
@@ -380,7 +382,8 @@ def load_standard_extensions(ext_mgr, logger, path, package, ext_list=None):
                     ext(ext_mgr)
                 except Exception as exc:
                     logger.warn(_('Failed to load extension %(ext_name)s: '
-                                  '%(exc)s') % locals())
+                                  '%(exc)s'),
+                                {'ext_name': ext_name, 'exc': exc})
 
         # Update the list of directories we'll explore...
         dirnames[:] = subdirs
index a3b2e6da2ecad6227b19c7098fefb8d25db1e864..2d62de8d851a46dbe5ddab0c35b28eb1eee0a144 100644 (file)
@@ -102,18 +102,20 @@ class APIRouter(base_wsgi.Router):
 
     def _setup_extensions(self, ext_mgr):
         for extension in ext_mgr.get_controller_extensions():
-            ext_name = extension.extension.name
             collection = extension.collection
             controller = extension.controller
 
             if collection not in self.resources:
                 LOG.warning(_('Extension %(ext_name)s: Cannot extend '
-                              'resource %(collection)s: No such resource') %
-                            locals())
+                              'resource %(collection)s: No such resource'),
+                            {'ext_name': extension.extension.name,
+                             'collection': collection})
                 continue
 
             LOG.debug(_('Extension %(ext_name)s extending resource: '
-                        '%(collection)s') % locals())
+                        '%(collection)s'),
+                      {'ext_name': extension.extension.name,
+                       'collection': collection})
 
             resource = self.resources[collection]
             resource.register_actions(controller)
index 83ff4be07ebe569afd93d5168ba899b2ebf6f4df..b9467a43a9f0989e11429945cfa88786d4e5e15d 100644 (file)
@@ -413,7 +413,8 @@ def remove_invalid_options(context, search_options, allowed_search_options):
     unknown_options = [opt for opt in search_options
                        if opt not in allowed_search_options]
     bad_options = ", ".join(unknown_options)
-    log_msg = _("Removing options '%(bad_options)s' from query") % locals()
+    log_msg = _("Removing options '%(bad_options)s'"
+                " from query") % {'bad_options': bad_options}
     LOG.debug(log_msg)
     for opt in unknown_options:
         del search_options[opt]
index 9a850fa07ef44d002319442b3175f928127673ad..a6149222b35fc0c14415df97e7f850973f5dcf83 100644 (file)
@@ -737,10 +737,10 @@ class MasterTemplate(Template):
 
             # Make sure we have a tree match
             if slave.root.tag != self.root.tag:
-                slavetag = slave.root.tag
-                mastertag = self.root.tag
-                msg = _("Template tree mismatch; adding slave %(slavetag)s "
-                        "to master %(mastertag)s") % locals()
+                msg = (_("Template tree mismatch; adding slave %(slavetag)s "
+                         "to master %(mastertag)s") %
+                       {'slavetag': slave.root.tag,
+                        'mastertag': self.root.tag})
                 raise ValueError(msg)
 
             # Make sure slave applies to this template