From 08aab97ced8763597329da56a64ef950f8567f4c Mon Sep 17 00:00:00 2001 From: Haomai Wang Date: Fri, 14 Jun 2013 15:11:29 +0800 Subject: [PATCH] Remove usage of locals() for formatting from cinder.api.* 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 | 13 ++++++++----- cinder/api/contrib/hosts.py | 3 ++- cinder/api/extensions.py | 9 ++++++--- cinder/api/openstack/__init__.py | 10 ++++++---- cinder/api/v1/volumes.py | 3 ++- cinder/api/xmlutil.py | 8 ++++---- 6 files changed, 28 insertions(+), 18 deletions(-) diff --git a/cinder/api/contrib/backups.py b/cinder/api/contrib/backups.py index 81c895372..ac4bd3552 100644 --- a/cinder/api/contrib/backups.py +++ b/cinder/api/contrib/backups.py @@ -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)) diff --git a/cinder/api/contrib/hosts.py b/cinder/api/contrib/hosts.py index 1a853f339..aedc8db6c 100644 --- a/cinder/api/contrib/hosts.py +++ b/cinder/api/contrib/hosts.py @@ -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) diff --git a/cinder/api/extensions.py b/cinder/api/extensions.py index 744adfcbd..da6042b65 100644 --- a/cinder/api/extensions.py +++ b/cinder/api/extensions.py @@ -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 diff --git a/cinder/api/openstack/__init__.py b/cinder/api/openstack/__init__.py index a3b2e6da2..2d62de8d8 100644 --- a/cinder/api/openstack/__init__.py +++ b/cinder/api/openstack/__init__.py @@ -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) diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index 83ff4be07..b9467a43a 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -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] diff --git a/cinder/api/xmlutil.py b/cinder/api/xmlutil.py index 9a850fa07..a6149222b 100644 --- a/cinder/api/xmlutil.py +++ b/cinder/api/xmlutil.py @@ -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 -- 2.45.2