diff --git a/HACKING.rst b/HACKING.rst index 146bc497..8dbfe564 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -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 diff --git a/ec2api/api/__init__.py b/ec2api/api/__init__.py index d783b47b..385ecb65 100644 --- a/ec2api/api/__init__.py +++ b/ec2api/api/__init__.py @@ -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) diff --git a/ec2api/api/apirequest.py b/ec2api/api/apirequest.py index 0b337356..bcd25855 100644 --- a/ec2api/api/apirequest.py +++ b/ec2api/api/apirequest.py @@ -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() diff --git a/ec2api/api/ec2utils.py b/ec2api/api/ec2utils.py index f5b34b65..1f0abb6f 100644 --- a/ec2api/api/ec2utils.py +++ b/ec2api/api/ec2utils.py @@ -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 diff --git a/ec2api/api/subnet.py b/ec2api/api/subnet.py index 068de775..ca1beeb2 100644 --- a/ec2api/api/subnet.py +++ b/ec2api/api/subnet.py @@ -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}) diff --git a/ec2api/api/volume.py b/ec2api/api/volume.py index 64256750..ea8ac649 100644 --- a/ec2api/api/volume.py +++ b/ec2api/api/volume.py @@ -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']) diff --git a/ec2api/api/vpc.py b/ec2api/api/vpc.py index 1babdb70..61847e41 100644 --- a/ec2api/api/vpc.py +++ b/ec2api/api/vpc.py @@ -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}) diff --git a/ec2api/api/vpn_connection.py b/ec2api/api/vpn_connection.py index ab5de36c..a5eff386 100644 --- a/ec2api/api/vpn_connection.py +++ b/ec2api/api/vpn_connection.py @@ -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}) diff --git a/ec2api/api/vpn_gateway.py b/ec2api/api/vpn_gateway.py index a5c207e2..c77fce06 100644 --- a/ec2api/api/vpn_gateway.py +++ b/ec2api/api/vpn_gateway.py @@ -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}) diff --git a/ec2api/exception.py b/ec2api/exception.py index 1e36d0d8..02cd7cdb 100644 --- a/ec2api/exception.py +++ b/ec2api/exception.py @@ -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')) diff --git a/ec2api/hacking/__init__.py b/ec2api/hacking/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/ec2api/hacking/checks.py b/ec2api/hacking/checks.py new file mode 100644 index 00000000..b6d3ad9f --- /dev/null +++ b/ec2api/hacking/checks.py @@ -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) diff --git a/ec2api/metadata/api.py b/ec2api/metadata/api.py index 68701d13..10a2bc93 100644 --- a/ec2api/metadata/api.py +++ b/ec2api/metadata/api.py @@ -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() diff --git a/ec2api/tests/unit/test_hacking.py b/ec2api/tests/unit/test_hacking.py new file mode 100644 index 00000000..3dd79396 --- /dev/null +++ b/ec2api/tests/unit/test_hacking.py @@ -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')))) diff --git a/ec2api/utils.py b/ec2api/utils.py index cf4a4673..c95c1ae9 100644 --- a/ec2api/utils.py +++ b/ec2api/utils.py @@ -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): diff --git a/ec2api/wsgi.py b/ec2api/wsgi.py index 8e1b47cb..961dae35 100644 --- a/ec2api/wsgi.py +++ b/ec2api/wsgi.py @@ -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: diff --git a/tox.ini b/tox.ini index 18387852..519c99d1 100644 --- a/tox.ini +++ b/tox.ini @@ -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