Remove usage of locals() for formatting from nova.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 fixes bug 1171936 Change-Id: I293d7ebb875f65cce322d4938d1ae323f3aded8d
This commit is contained in:
parent
f124d50024
commit
2dce8c92f6
@ -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']
|
||||
|
||||
|
@ -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
|
||||
|
@ -78,7 +78,7 @@ CONF.import_opt('use_forwarded_for', 'nova.api.auth')
|
||||
|
||||
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 @@ class Lockout(wsgi.Middleware):
|
||||
# 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 @@ class Requestify(wsgi.Middleware):
|
||||
|
||||
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 @@ class Authorizer(wsgi.Middleware):
|
||||
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):
|
||||
|
@ -57,11 +57,10 @@ class APIRequest(object):
|
||||
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()
|
||||
|
@ -621,8 +621,7 @@ class CloudController(object):
|
||||
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,
|
||||
@ -876,9 +875,12 @@ class CloudController(object):
|
||||
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,
|
||||
@ -1238,7 +1240,7 @@ class CloudController(object):
|
||||
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"}
|
||||
@ -1246,8 +1248,10 @@ class CloudController(object):
|
||||
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)
|
||||
|
||||
@ -1506,9 +1510,10 @@ class CloudController(object):
|
||||
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):
|
||||
@ -1615,10 +1620,10 @@ class CloudController(object):
|
||||
# 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
|
||||
|
@ -166,10 +166,14 @@ class MetadataRequestHandler(wsgi.Application):
|
||||
|
||||
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)
|
||||
|
@ -174,18 +174,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
|
||||
|
||||
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)
|
||||
|
@ -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):
|
||||
|
@ -289,11 +289,11 @@ class AdminActionsController(wsgi.Controller):
|
||||
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)
|
||||
|
@ -86,7 +86,7 @@ class AggregateController(object):
|
||||
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 @@ class AggregateController(object):
|
||||
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 @@ class AggregateController(object):
|
||||
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 @@ class AggregateController(object):
|
||||
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 @@ class AggregateController(object):
|
||||
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 @@ class AggregateController(object):
|
||||
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 @@ class AggregateController(object):
|
||||
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)
|
||||
|
@ -309,7 +309,7 @@ class FloatingIPActionController(wsgi.Controller):
|
||||
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)
|
||||
|
||||
|
||||
|
@ -202,8 +202,9 @@ class HostController(object):
|
||||
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:
|
||||
|
@ -397,9 +397,12 @@ class VolumeAttachmentController(wsgi.Controller):
|
||||
|
||||
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)
|
||||
|
@ -1404,9 +1404,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)
|
||||
|
@ -275,7 +275,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):
|
||||
@ -344,19 +345,18 @@ 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 = []
|
||||
for dname in dirnames:
|
||||
# Skip it if it does not have __init__.py
|
||||
if not os.path.exists(os.path.join(dirpath, dname,
|
||||
'__init__.py')):
|
||||
if not os.path.exists(os.path.join(dirpath, dname, '__init__.py')):
|
||||
continue
|
||||
|
||||
# If it has extension(), delegate...
|
||||
ext_name = ("%s%s.%s.extension" %
|
||||
(package, relpkg, dname))
|
||||
ext_name = "%s%s.%s.extension" % (package, relpkg, dname)
|
||||
try:
|
||||
ext = importutils.import_class(ext_name)
|
||||
except ImportError:
|
||||
@ -367,8 +367,9 @@ def load_standard_extensions(ext_mgr, logger, path, package, ext_list=None):
|
||||
try:
|
||||
ext(ext_mgr)
|
||||
except Exception as exc:
|
||||
logger.warn(_('Failed to load extension %(ext_name)s: '
|
||||
'%(exc)s') % locals())
|
||||
logger.warn(_('Failed to load extension %(ext_name)s:'
|
||||
'%(exc)s'),
|
||||
{'ext_name': ext_name, 'exc': exc})
|
||||
|
||||
# Update the list of directories we'll explore...
|
||||
dirnames[:] = subdirs
|
||||
|
@ -916,7 +916,9 @@ class Resource(wsgi.Application):
|
||||
return Fault(webob.exc.HTTPBadRequest(explanation=msg))
|
||||
|
||||
if body:
|
||||
LOG.debug(_("Action: '%(action)s', body: %(body)s") % locals())
|
||||
msg = _("Action: '%(action)s', body: "
|
||||
"%(body)s") % {'action': action, 'body': body}
|
||||
LOG.debug(msg)
|
||||
LOG.debug(_("Calling method %s") % meth)
|
||||
|
||||
# Now, deserialize the request body...
|
||||
|
@ -739,10 +739,9 @@ 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
|
||||
|
Loading…
Reference in New Issue
Block a user