Merge "Remove usage of locals() for formatting from nova.api.*"

doc/ext/nova_todo.py

@@ -37,13 +37,13 @@ def process_todo_nodes(app, doctree, fromdocname):
 
         for todo_info in env.todo_all_todos:
             para = nodes.paragraph()
-            filename = env.doc2path(todo_info['docname'], base=None)
-
             # Create a reference
             newnode = nodes.reference('', '')
 
-            line_info = todo_info['lineno']
-            link = _('%(filename)s, line %(line_info)d') % locals()
+            filename = env.doc2path(todo_info['docname'], base=None)
+            link = (_('%(filename)s, line %(line_info)d') %
+                     {'filename': filename, 'line_info': todo_info['lineno']})
+
             innernode = nodes.emphasis(link, link)
             newnode['refdocname'] = todo_info['docname']
 

doc/source/devref/il8n.rst

@@ -9,14 +9,11 @@ in a ``_()`` function call. For example::
 
     LOG.debug(_("block_device_mapping %s"), block_device_mapping)
 
-If you have multiple arguments, the convention is to use named parameters.
-It's common to use the ``locals()`` dict (which contains the names and values
-of the local variables in the current scope) to do the string interpolation.
-For example::
-
-    label = ...
-    sr_ref = ...
-    LOG.debug(_('Introduced %(label)s as %(sr_ref)s.') % locals())
+Do not use ``locals()`` for formatting messages because:
+1. It is not as clear as using explicit dicts.
+2. It could produce hidden errors during refactoring.
+3. Changing the name of a variable causes a change in the message.
+4. It creates a lot of otherwise unused variables.
 
 If you do not follow the project conventions, your code may cause the
 LocalizationTestCase.test_multiple_positional_format_placeholders test to fail

nova/api/ec2/__init__.py

@@ -78,7 +78,7 @@
 
 def ec2_error(req, request_id, code, message):
     """Helper to send an ec2_compatible error."""
-    LOG.error(_('%(code)s: %(message)s') % locals())
+    LOG.error(_('%(code)s: %(message)s'), {'code': code, 'message': message})
     resp = webob.Response()
     resp.status = 400
     resp.headers['Content-Type'] = 'text/xml'
@@ -180,11 +180,12 @@ def __call__(self, req):
                 # NOTE(vish): To use incr, failures has to be a string.
                 self.mc.set(failures_key, '1', time=CONF.lockout_window * 60)
             elif failures >= CONF.lockout_attempts:
-                lock_mins = CONF.lockout_minutes
-                msg = _('Access key %(access_key)s has had %(failures)d'
-                        ' failed authentications and will be locked out'
-                        ' for %(lock_mins)d minutes.') % locals()
-                LOG.warn(msg)
+                LOG.warn(_('Access key %(access_key)s has had %(failures)d '
+                           'failed authentications and will be locked out '
+                           'for %(lock_mins)d minutes.'),
+                         {'access_key': access_key,
+                          'failures': failures,
+                          'lock_mins': CONF.lockout_minutes})
                 self.mc.set(failures_key, str(failures),
                             time=CONF.lockout_minutes * 60)
         return res
@@ -333,7 +334,8 @@ def __call__(self, req):
 
         LOG.debug(_('action: %s'), action)
         for key, value in args.items():
-            LOG.debug(_('arg: %(key)s\t\tval: %(value)s') % locals())
+            LOG.debug(_('arg: %(key)s\t\tval: %(value)s'),
+                      {'key': key, 'value': value})
 
         # Success!
         api_request = apirequest.APIRequest(self.controller, action,
@@ -409,7 +411,9 @@ def __call__(self, req):
             return self.application
         else:
             LOG.audit(_('Unauthorized request for controller=%(controller)s '
-                        'and action=%(action)s') % locals(), context=context)
+                        'and action=%(action)s'),
+                      {'controller': controller, 'action': action},
+                      context=context)
             raise webob.exc.HTTPUnauthorized()
 
     def _matches_any_role(self, context, roles):

nova/api/ec2/apirequest.py

@@ -57,11 +57,10 @@ def invoke(self, context):
             method = getattr(self.controller,
                              ec2utils.camelcase_to_underscore(self.action))
         except AttributeError:
-            controller = self.controller
-            action = self.action
-            _error = _('Unsupported API request: controller = %(controller)s,'
-                    ' action = %(action)s') % locals()
-            LOG.exception(_error)
+            LOG.exception(_('Unsupported API request: controller = '
+                            '%(controller)s, action = %(action)s'),
+                          {'controller': self.controller,
+                           'action': self.action})
             # TODO(gundlach): Raise custom exception, trap in apiserver,
             #       and reraise as 400 error.
             raise exception.InvalidRequest()

nova/api/ec2/cloud.py

@@ -620,8 +620,7 @@ def _validate_security_group_protocol(self, values):
         validprotocols = ['tcp', 'udp', 'icmp', '6', '17', '1']
         if 'ip_protocol' in values and \
             values['ip_protocol'] not in validprotocols:
-            protocol = values['ip_protocol']
-            err = _("Invalid IP protocol %(protocol)s.") % locals()
+            err = _('Invalid IP protocol %s.') % values['ip_protocol']
             raise exception.EC2APIError(message=err, code="400")
 
     def revoke_security_group_ingress(self, context, group_name=None,
@@ -874,9 +873,12 @@ def attach_volume(self, context,
         volume_id = ec2utils.ec2_vol_id_to_uuid(volume_id)
         instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, instance_id)
         instance = self.compute_api.get(context, instance_uuid)
-        msg = _("Attach volume %(volume_id)s to instance %(instance_id)s"
-                " at %(device)s") % locals()
-        LOG.audit(msg, context=context)
+        LOG.audit(_('Attach volume %(volume_id)s to instance %(instance_id)s '
+                    'at %(device)s'),
+                  {'volume_id': volume_id,
+                   'instance_id': instance_id,
+                   'device': device},
+                  context=context)
 
         try:
             self.compute_api.attach_volume(context, instance,
@@ -1236,16 +1238,18 @@ def allocate_address(self, context, **kwargs):
         return {'publicIp': public_ip}
 
     def release_address(self, context, public_ip, **kwargs):
-        LOG.audit(_("Release address %s"), public_ip, context=context)
+        LOG.audit(_('Release address %s'), public_ip, context=context)
         try:
             self.network_api.release_floating_ip(context, address=public_ip)
             return {'return': "true"}
         except exception.FloatingIpNotFound:
             raise exception.EC2APIError(_('Unable to release IP Address.'))
 
     def associate_address(self, context, instance_id, public_ip, **kwargs):
-        LOG.audit(_("Associate address %(public_ip)s to"
-                " instance %(instance_id)s") % locals(), context=context)
+        LOG.audit(_("Associate address %(public_ip)s to instance "
+                    "%(instance_id)s"),
+                  {'public_ip': public_ip, 'instance_id': instance_id},
+                  context=context)
         instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, instance_id)
         instance = self.compute_api.get(context, instance_uuid)
 
@@ -1504,9 +1508,10 @@ def register_image(self, context, image_location=None, **kwargs):
             metadata['properties']['block_device_mapping'] = mappings
 
         image_id = self._register_image(context, metadata)
-        msg = _("Registered image %(image_location)s with"
-                " id %(image_id)s") % locals()
-        LOG.audit(msg, context=context)
+        LOG.audit(_('Registered image %(image_location)s with id '
+                    '%(image_id)s'),
+                  {'image_location': image_location, 'image_id': image_id},
+                  context=context)
         return {'imageId': image_id}
 
     def describe_image_attribute(self, context, image_id, attribute, **kwargs):
@@ -1613,10 +1618,10 @@ def create_image(self, context, instance_id, **kwargs):
         # CreateImage only supported for the analogue of EBS-backed instances
         if not self.compute_api.is_volume_backed_instance(context, instance,
                                                           bdms):
-            root = instance['root_device_name']
             msg = _("Invalid value '%(ec2_instance_id)s' for instanceId. "
                     "Instance does not have a volume attached at root "
-                    "(%(root)s)") % locals()
+                    "(%(root)s)") % {'root': instance['root_device_name'],
+                                     'ec2_instance_id': ec2_instance_id}
             raise exception.InvalidParameterValue(err=msg)
 
         # stop the instance if necessary

nova/api/metadata/handler.py

@@ -166,10 +166,14 @@ def _handle_instance_id_request(self, req):
 
         if expected_signature != signature:
             if instance_id:
-                w = _('X-Instance-ID-Signature: %(signature)s does not match '
-                      'the expected value: %(expected_signature)s for id: '
-                      '%(instance_id)s.  Request From: %(remote_address)s')
-                LOG.warn(w % locals())
+                LOG.warn(_('X-Instance-ID-Signature: %(signature)s does not '
+                           'match the expected value: %(expected_signature)s '
+                           'for id: %(instance_id)s.  Request From: '
+                            '%(remote_address)s'),
+                         {'signature': signature,
+                          'expected_signature': expected_signature,
+                          'instance_id': instance_id,
+                          'remote_address': remote_address})
 
             msg = _('Invalid proxy request signature.')
             raise webob.exc.HTTPForbidden(explanation=msg)

nova/api/openstack/__init__.py

@@ -174,18 +174,20 @@ def _setup_ext_routes(self, mapper, ext_mgr, init_only):
 
     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
 
+            msg_format_dict = {'collection': collection,
+                               'ext_name': extension.extension.name}
             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'),
+                            msg_format_dict)
                 continue
 
             LOG.debug(_('Extension %(ext_name)s extending resource: '
-                        '%(collection)s') % locals())
+                        '%(collection)s'),
+                      msg_format_dict)
 
             resource = self.resources[collection]
             resource.register_actions(controller)

nova/api/openstack/common.py

@@ -113,7 +113,8 @@ def status_from_state(vm_state, task_state='default'):
     if status == "UNKNOWN":
         LOG.error(_("status is UNKNOWN from vm_state=%(vm_state)s "
                     "task_state=%(task_state)s. Bad upgrade or db "
-                    "corrupted?") % locals())
+                    "corrupted?"),
+                  {'vm_state': vm_state, 'task_state': task_state})
     return status
 
 
@@ -358,11 +359,12 @@ def raise_http_conflict_for_instance_invalid_state(exc, action):
     attr = exc.kwargs.get('attr')
     state = exc.kwargs.get('state')
     if attr and state:
-        msg = _("Cannot '%(action)s' while instance is in %(attr)s %(state)s")
+        msg = _("Cannot '%(action)s' while instance is in %(attr)s "
+                "%(state)s") % {'action': action, 'attr': attr, 'state': state}
     else:
         # At least give some meaningful message
-        msg = _("Instance is in an invalid state for '%(action)s'")
-    raise webob.exc.HTTPConflict(explanation=msg % locals())
+        msg = _("Instance is in an invalid state for '%s'") % action
+    raise webob.exc.HTTPConflict(explanation=msg)
 
 
 class MetadataDeserializer(wsgi.MetadataXMLDeserializer):

nova/api/openstack/compute/contrib/admin_actions.py

@@ -289,11 +289,11 @@ def _migrate_live(self, req, id, body):
             raise exc.HTTPBadRequest(explanation=ex.format_message())
         except Exception:
             if host is None:
-                msg = _("Live migration of instance %(id)s to another host"
-                        " failed") % locals()
+                msg = _("Live migration of instance %s to another host "
+                        "failed") % id
             else:
-                msg = _("Live migration of instance %(id)s to host %(host)s"
-                        " failed") % locals()
+                msg = _("Live migration of instance %(id)s to host %(host)s "
+                        "failed") % {'id': id, 'host': host}
             LOG.exception(msg)
             # Return messages from scheduler
             raise exc.HTTPBadRequest(explanation=msg)

nova/api/openstack/compute/contrib/aggregates.py

@@ -86,7 +86,7 @@ def show(self, req, id):
         try:
             aggregate = self.api.get_aggregate(context, id)
         except exception.AggregateNotFound:
-            LOG.info(_("Cannot show aggregate: %(id)s") % locals())
+            LOG.info(_("Cannot show aggregate: %s"), id)
             raise exc.HTTPNotFound
         return self._marshall_aggregate(aggregate)
 
@@ -112,7 +112,7 @@ def update(self, req, id, body):
         try:
             aggregate = self.api.update_aggregate(context, id, updates)
         except exception.AggregateNotFound:
-            LOG.info(_("Cannot update aggregate: %(id)s") % locals())
+            LOG.info(_('Cannot update aggregate: %s'), id)
             raise exc.HTTPNotFound
 
         return self._marshall_aggregate(aggregate)
@@ -124,7 +124,7 @@ def delete(self, req, id):
         try:
             self.api.delete_aggregate(context, id)
         except exception.AggregateNotFound:
-            LOG.info(_("Cannot delete aggregate: %(id)s") % locals())
+            LOG.info(_('Cannot delete aggregate: %s'), id)
             raise exc.HTTPNotFound
 
     def action(self, req, id, body):
@@ -137,7 +137,7 @@ def action(self, req, id, body):
             try:
                 return _actions[action](req, id, data)
             except KeyError:
-                msg = _("Aggregates does not have %s action") % action
+                msg = _('Aggregates does not have %s action') % action
                 raise exc.HTTPBadRequest(explanation=msg)
 
         raise exc.HTTPBadRequest(explanation=_("Invalid request body"))
@@ -150,13 +150,13 @@ def _add_host(self, req, id, host):
         try:
             aggregate = self.api.add_host_to_aggregate(context, id, host)
         except (exception.AggregateNotFound, exception.ComputeHostNotFound):
-            LOG.info(_("Cannot add host %(host)s in aggregate "
-                            "%(id)s") % locals())
+            LOG.info(_('Cannot add host %(host)s in aggregate %(id)s'),
+                     {'host': host, 'id': id})
             raise exc.HTTPNotFound
         except (exception.AggregateHostExists,
                 exception.InvalidAggregateAction):
-            LOG.info(_("Cannot add host %(host)s in aggregate "
-                            "%(id)s") % locals())
+            LOG.info(_('Cannot add host %(host)s in aggregate %(id)s'),
+                     {'host': host, 'id': id})
             raise exc.HTTPConflict
         return self._marshall_aggregate(aggregate)
 
@@ -169,12 +169,12 @@ def _remove_host(self, req, id, host):
             aggregate = self.api.remove_host_from_aggregate(context, id, host)
         except (exception.AggregateNotFound, exception.AggregateHostNotFound,
                 exception.ComputeHostNotFound):
-            LOG.info(_("Cannot remove host %(host)s in aggregate "
-                            "%(id)s") % locals())
+            LOG.info(_('Cannot remove host %(host)s in aggregate %(id)s'),
+                     {'host': host, 'id': id})
             raise exc.HTTPNotFound
         except exception.InvalidAggregateAction:
-            LOG.info(_("Cannot remove host %(host)s in aggregate "
-                "%(id)s") % locals())
+            LOG.info(_('Cannot remove host %(host)s in aggregate %(id)s'),
+                     {'host': host, 'id': id})
             raise exc.HTTPConflict
         return self._marshall_aggregate(aggregate)
 
@@ -193,8 +193,8 @@ def _set_metadata(self, req, id, body):
             aggregate = self.api.update_aggregate_metadata(context,
                                                            id, metadata)
         except exception.AggregateNotFound:
-            LOG.info(_("Cannot set metadata %(metadata)s in aggregate "
-                            "%(id)s") % locals())
+            LOG.info(_('Cannot set metadata %(metadata)s in aggregate %(id)s'),
+                     {'metadata': metadata, 'id': id})
             raise exc.HTTPNotFound
 
         return self._marshall_aggregate(aggregate)

nova/api/openstack/compute/contrib/floating_ips.py

@@ -309,7 +309,7 @@ def _remove_floating_ip(self, req, id, body):
             return webob.Response(status_int=202)
         else:
             msg = _("Floating ip %(address)s is not associated with instance "
-                    "%(id)s.") % locals()
+                    "%(id)s.") % {'address': address, 'id': id}
             raise webob.exc.HTTPUnprocessableEntity(explanation=msg)
 
 

nova/api/openstack/compute/contrib/hosts.py

@@ -202,8 +202,9 @@ def read_enabled(orig_val, msg):
     def _set_host_maintenance(self, context, host_name, mode=True):
         """Start/Stop host maintenance window. On start, it triggers
         guest VMs evacuation."""
-        LOG.audit(_("Putting host %(host_name)s in maintenance "
-                    "mode %(mode)s.") % locals())
+        LOG.audit(_("Putting host %(host_name)s in maintenance mode "
+                    "%(mode)s."),
+                  {'host_name': host_name, 'mode': mode})
         try:
             result = self.api.set_host_maintenance(context, host_name, mode)
         except NotImplementedError:

nova/api/openstack/compute/contrib/volumes.py

@@ -397,9 +397,12 @@ def create(self, req, server_id, body):
 
         self._validate_volume_id(volume_id)
 
-        msg = _("Attach volume %(volume_id)s to instance %(server_id)s"
-                " at %(device)s") % locals()
-        LOG.audit(msg, context=context)
+        LOG.audit(_("Attach volume %(volume_id)s to instance %(server_id)s "
+                    "at %(device)s"),
+                  {'volume_id': volume_id,
+                   'device': device,
+                   'server_id': server_id},
+                  context=context)
 
         try:
             instance = self.compute_api.get(context, server_id)

nova/api/openstack/compute/servers.py

@@ -1406,9 +1406,8 @@ def remove_invalid_options(context, search_options, allowed_search_options):
         return
     # Otherwise, strip out all unknown options
     unknown_options = [opt for opt in search_options
-            if opt not in allowed_search_options]
-    unk_opt_str = ", ".join(unknown_options)
-    log_msg = _("Removing options '%(unk_opt_str)s' from query") % locals()
-    LOG.debug(log_msg)
+                        if opt not in allowed_search_options]
+    LOG.debug(_("Removing options '%s' from query"),
+              ", ".join(unknown_options))
     for opt in unknown_options:
         search_options.pop(opt, None)