Delete log translation functions and add hacking rule

The i18n team has decided not to translate the logs because it seems
like it not very useful; operators prefer to have them in English so
that they can search for those strings on the internet.

Since we have removed log translations completely, we should add hacking
rule to prevent future mistakes.

Change-Id: Ia7524308ef2675f8d41ac80b37dfc7e3787efd90
This commit is contained in:
Ngo Quoc Cuong 2017-07-03 04:13:46 -04:00
parent 0e3627252e
commit 90ed02d682
17 changed files with 119 additions and 40 deletions

View File

@ -12,10 +12,10 @@ General
-------
- Do not use locals(). Example::
LOG.debug(_("volume %(vol_name)s: creating size %(vol_size)sG") %
LOG.debug("volume %(vol_name)s: creating size %(vol_size)sG" %
locals()) # BAD
LOG.debug(_("volume %(vol_name)s: creating size %(vol_size)sG") %
LOG.debug("volume %(vol_name)s: creating size %(vol_size)sG" %
{'vol_name': vol_name,
'vol_size': vol_size}) # OKAY

View File

@ -75,7 +75,7 @@ class FaultWrapper(wsgi.Middleware):
try:
return req.get_response(self.application)
except Exception:
LOG.exception(_("FaultWrapper catches error"))
LOG.exception("FaultWrapper catches error")
return faults.Fault(webob.exc.HTTPInternalServerError())
@ -226,7 +226,7 @@ class EC2KeystoneAuth(wsgi.Middleware):
auth_ref = keystone_access.AccessInfo.factory(resp=response,
body=response.json())
except (NotImplementedError, KeyError):
LOG.exception(_("Keystone failure"))
LOG.exception("Keystone failure")
msg = _("Failure communicating with keystone")
return faults.ec2_error_response(request_id, "AuthFailure", msg,
status=400)

View File

@ -24,7 +24,7 @@ import six
from ec2api.api import cloud
from ec2api.api import ec2utils
from ec2api import exception
from ec2api.i18n import _
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
@ -52,7 +52,7 @@ class APIRequest(object):
method = getattr(self.controller,
ec2utils.camelcase_to_underscore(self.action))
except AttributeError:
LOG.exception(_('Unsupported API request: action = %(action)s'),
LOG.exception('Unsupported API request: action = %(action)s',
{'action': self.action})
raise exception.InvalidRequest()

View File

@ -203,7 +203,7 @@ def is_ec2_timestamp_expired(request, expires=None):
timeutils.is_newer_than(query_time, expires))
return False
except ValueError:
LOG.exception(_("Timestamp is invalid: "))
LOG.exception("Timestamp is invalid: ")
return True

View File

@ -127,8 +127,8 @@ def delete_subnet(context, subnet_id):
try:
neutron.delete_network(os_subnet['network_id'])
except neutron_exception.NetworkInUseClient as ex:
LOG.warning(_('Failed to delete network %(os_id)s during '
'deleting Subnet %(id)s. Reason: %(reason)s'),
LOG.warning('Failed to delete network %(os_id)s during '
'deleting Subnet %(id)s. Reason: %(reason)s',
{'id': subnet['id'],
'os_id': os_subnet['network_id'],
'reason': ex.message})

View File

@ -68,7 +68,7 @@ def attach_volume(context, volume_id, instance_id, device):
device)
except (nova_exception.Conflict, nova_exception.BadRequest):
# TODO(andrey-mp): raise correct errors for different cases
LOG.exception(_('Attach has failed.'))
LOG.exception('Attach has failed.')
raise exception.UnsupportedOperation()
cinder = clients.cinder(context)
os_volume = cinder.volumes.get(volume['os_id'])

View File

@ -89,8 +89,8 @@ def delete_vpc(context, vpc_id):
try:
neutron.delete_router(vpc['os_id'])
except neutron_exception.Conflict as ex:
LOG.warning(_('Failed to delete router %(os_id)s during deleting '
'VPC %(id)s. Reason: %(reason)s'),
LOG.warning('Failed to delete router %(os_id)s during deleting '
'VPC %(id)s. Reason: %(reason)s',
{'id': vpc['id'],
'os_id': vpc['os_id'],
'reason': ex.message})

View File

@ -165,20 +165,19 @@ def delete_vpn_connection(context, vpn_connection_id):
try:
neutron.delete_ipsecpolicy(vpn_connection['os_ipsecpolicy_id'])
except neutron_exception.Conflict as ex:
LOG.warning(
_('Failed to delete ipsecoplicy %(os_id)s during deleting '
'VPN connection %(id)s. Reason: %(reason)s'),
{'id': vpn_connection['id'],
'os_id': vpn_connection['os_ipsecpolicy_id'],
'reason': ex.message})
LOG.warning('Failed to delete ipsecoplicy %(os_id)s during '
'deleting VPN connection %(id)s. Reason: %(reason)s',
{'id': vpn_connection['id'],
'os_id': vpn_connection['os_ipsecpolicy_id'],
'reason': ex.message})
except neutron_exception.NotFound:
pass
try:
neutron.delete_ikepolicy(vpn_connection['os_ikepolicy_id'])
except neutron_exception.Conflict as ex:
LOG.warning(
_('Failed to delete ikepolicy %(os_id)s during deleting '
'VPN connection %(id)s. Reason: %(reason)s'),
'Failed to delete ikepolicy %(os_id)s during deleting '
'VPN connection %(id)s. Reason: %(reason)s',
{'id': vpn_connection['id'],
'os_id': vpn_connection['os_ikepolicy_id'],
'reason': ex.message})

View File

@ -198,8 +198,8 @@ def _safe_delete_vpnservice(neutron, os_vpnservice_id, subnet_id):
pass
except neutron_exception.Conflict as ex:
LOG.warning(
_('Failed to delete vpnservice %(os_id)s for subnet %(id)s. '
'Reason: %(reason)s'),
'Failed to delete vpnservice %(os_id)s for subnet %(id)s. '
'Reason: %(reason)s',
{'id': subnet_id,
'os_id': os_vpnservice_id,
'reason': ex.message})

View File

@ -59,8 +59,8 @@ class EC2APIException(Exception):
exc_info = sys.exc_info()
# kwargs doesn't match a variable in the message
# log the issue and the kwargs
LOG.exception(_('Exception in string format operation for '
'%s exception'), self.__class__.__name__)
LOG.exception('Exception in string format operation for '
'%s exception', self.__class__.__name__)
for name, value in kwargs.items():
LOG.error('%s: %s' % (name, value))
@ -70,8 +70,8 @@ class EC2APIException(Exception):
# at least get the core message out if something happened
message = self.msg_fmt
elif not isinstance(message, six.string_types):
LOG.error(_("Message '%(msg)s' for %(ex)s exception is not "
"a string"),
LOG.error("Message '%(msg)s' for %(ex)s exception is not "
"a string",
{'msg': message, 'ex': self.__class__.__name__})
if CONF.fatal_exception_format_errors:
raise TypeError(_('Invalid exception message format'))

View File

52
ec2api/hacking/checks.py Normal file
View File

@ -0,0 +1,52 @@
# Copyright (c) 2017 OpenStack Foundation.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import re
_all_log_levels = {'critical', 'error', 'exception', 'info',
'warning', 'debug'}
# Since _Lx() have been removed, we just need to check _()
_all_hints = {'_'}
_log_translation_hint = re.compile(
r".*LOG\.(%(levels)s)\(\s*(%(hints)s)\(" % {
'levels': '|'.join(_all_log_levels),
'hints': '|'.join(_all_hints),
})
def no_translate_logs(logical_line, filename):
"""N537 - Don't translate logs.
Check for 'LOG.*(_('
Translators don't provide translations for log messages, and operators
asked not to translate them.
* This check assumes that 'LOG' is a logger.
:param logical_line: The logical line to check.
:param filename: The file name where the logical line exists.
:returns: None if the logical line passes the check, otherwise a tuple
is yielded that contains the offending index in logical line and a
message describe the check validation failure.
"""
if _log_translation_hint.match(logical_line):
yield (0, "N537: Log messages should not be translated!")
def factory(register):
register(no_translate_logs)

View File

@ -25,7 +25,7 @@ from ec2api.api import clients
from ec2api.api import ec2utils
from ec2api.api import instance as instance_api
from ec2api import exception
from ec2api.i18n import _
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
@ -137,7 +137,7 @@ def _get_ec2_instance_and_reservation(context, os_instance_id):
'value': [instance_id]}])
if (len(ec2_reservations['reservationSet']) != 1 or
len(ec2_reservations['reservationSet'][0]['instancesSet']) != 1):
LOG.error(_('Failed to get metadata for instance id: %s'),
LOG.error('Failed to get metadata for instance id: %s',
os_instance_id)
raise exception.EC2MetadataNotFound()
@ -152,8 +152,8 @@ def _check_instance_owner(context, os_instance_id, owner_id):
# It sends project_id as X-Tenant-ID HTTP header.
# We make sure it's correct
if context.project_id != owner_id:
LOG.warning(_('Tenant_id %(tenant_id)s does not match tenant_id '
'of instance %(instance_id)s.'),
LOG.warning('Tenant_id %(tenant_id)s does not match tenant_id '
'of instance %(instance_id)s.',
{'tenant_id': context.project_id,
'instance_id': os_instance_id})
raise exception.EC2MetadataNotFound()

View File

@ -0,0 +1,29 @@
# Copyright (c) 2017 OpenStack Foundation.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from ec2api.hacking import checks
from ec2api.tests.unit import base
class HackingTestCase(base.BaseTestCase):
def test_no_log_translations(self):
for log in checks._all_log_levels:
for hint in checks._all_hints:
bad = 'LOG.%s(%s("Bad"))' % (log, hint)
self.assertEqual(
1, len(list(checks.no_translate_logs(bad, 'f'))))
# Catch abuses when used with a variable and not a literal
bad = 'LOG.%s(%s(msg))' % (log, hint)
self.assertEqual(
1, len(list(checks.no_translate_logs(bad, 'f'))))

View File

@ -25,7 +25,6 @@ from xml.sax import saxutils
from oslo_config import cfg
from oslo_log import log as logging
from ec2api.i18n import _
utils_opts = [
cfg.StrOpt('tempdir',
@ -49,7 +48,7 @@ def tempdir(**kwargs):
try:
shutil.rmtree(tmpdir)
except OSError as e:
LOG.error(_('Could not remove tmpdir: %s'), str(e))
LOG.error('Could not remove tmpdir: %s', str(e))
def get_hash_str(base_str):

View File

@ -124,12 +124,12 @@ class Server(ServiceBase):
try:
self._socket = eventlet.listen(bind_addr, family, backlog=backlog)
except EnvironmentError:
LOG.error(_("Could not bind to %(host)s:%(port)s"),
LOG.error("Could not bind to %(host)s:%(port)s",
{'host': host, 'port': port})
raise
(self.host, self.port) = self._socket.getsockname()[0:2]
LOG.info(_("%(name)s listening on %(host)s:%(port)s"),
LOG.info("%(name)s listening on %(host)s:%(port)s",
{'name': self.name, 'host': self.host, 'port': self.port})
def start(self):
@ -184,8 +184,8 @@ class Server(ServiceBase):
**ssl_kwargs)
except Exception:
with excutils.save_and_reraise_exception():
LOG.error(_("Failed to start %(name)s on %(host)s"
":%(port)s with SSL support"),
LOG.error("Failed to start %(name)s on %(host)s"
":%(port)s with SSL support",
{'name': self.name, 'host': self.host,
'port': self.port})
@ -222,7 +222,7 @@ class Server(ServiceBase):
:returns: None
"""
LOG.info(_("Stopping WSGI server."))
LOG.info("Stopping WSGI server.")
if self._server is not None:
# Resize pool to stop new requests from being processed
@ -241,7 +241,7 @@ class Server(ServiceBase):
if self._server is not None:
self._server.wait()
except greenlet.GreenletExit:
LOG.info(_("WSGI server has stopped."))
LOG.info("WSGI server has stopped.")
class Request(webob.Request):
@ -499,7 +499,7 @@ class Loader(object):
"""
try:
LOG.debug(_("Loading app %(name)s from %(path)s") %
LOG.debug("Loading app %(name)s from %(path)s",
{'name': name, 'path': self.config_path})
return deploy.loadapp("config:%s" % self.config_path, name=name)
except LookupError as err:

View File

@ -54,7 +54,7 @@ exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build,tools,ec2api/tests/f
max-complexity=25
[hacking]
import_exceptions = ec2api.i18n
local-check-factory = ec2api.hacking.checks.factory
[testenv:install-guide]
commands = sphinx-build -a -E -W -d install-guide/build/doctrees -b html install-guide/source install-guide/build/html