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

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
 

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)

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()
 

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
 
 

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 '
+                LOG.warning('Failed to delete network %(os_id)s during '
-                              'deleting Subnet %(id)s. Reason: %(reason)s'),
+                            'deleting Subnet %(id)s. Reason: %(reason)s',
                             {'id': subnet['id'],
                              'os_id': os_subnet['network_id'],
                              'reason': ex.message})

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'])

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 '
+            LOG.warning('Failed to delete router %(os_id)s during deleting '
-                          'VPC %(id)s. Reason: %(reason)s'),
+                        'VPC %(id)s. Reason: %(reason)s',
                         {'id': vpc['id'],
                          'os_id': vpc['os_id'],
                          'reason': ex.message})

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(
+            LOG.warning('Failed to delete ipsecoplicy %(os_id)s during '
-                _('Failed to delete ipsecoplicy %(os_id)s during deleting '
+                        'deleting VPN connection %(id)s. Reason: %(reason)s',
-                  'VPN connection %(id)s. Reason: %(reason)s'),
+                        {'id': vpn_connection['id'],
-                {'id': vpn_connection['id'],
+                         'os_id': vpn_connection['os_ipsecpolicy_id'],
-                 'os_id': vpn_connection['os_ipsecpolicy_id'],
+                         'reason': ex.message})
-                 '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 '
+                'Failed to delete ikepolicy %(os_id)s during deleting '
-                  'VPN connection %(id)s. Reason: %(reason)s'),
+                'VPN connection %(id)s. Reason: %(reason)s',
                 {'id': vpn_connection['id'],
                  'os_id': vpn_connection['os_ikepolicy_id'],
                  'reason': ex.message})

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. '
+            'Failed to delete vpnservice %(os_id)s for subnet %(id)s. '
-              'Reason: %(reason)s'),
+            'Reason: %(reason)s',
             {'id': subnet_id,
              'os_id': os_vpnservice_id,
              'reason': ex.message})

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 '
+                LOG.exception('Exception in string format operation for '
-                                '%s exception'), self.__class__.__name__)
+                              '%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 "
+            LOG.error("Message '%(msg)s' for %(ex)s exception is not "
-                        "a string"),
+                      "a string",
                       {'msg': message, 'ex': self.__class__.__name__})
             if CONF.fatal_exception_format_errors:
                 raise TypeError(_('Invalid exception message format'))

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)

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 '
+        LOG.warning('Tenant_id %(tenant_id)s does not match tenant_id '
-                      'of instance %(instance_id)s.'),
+                    'of instance %(instance_id)s.',
                     {'tenant_id': context.project_id,
                      'instance_id': os_instance_id})
         raise exception.EC2MetadataNotFound()

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'))))

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):

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"
+                    LOG.error("Failed to start %(name)s on %(host)s"
-                                ":%(port)s with SSL support"),
+                              ":%(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:

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