From 1d3d04150cba863cefcbe3a37a065b63f401d5c2 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Cuong Date: Mon, 3 Jul 2017 03:37:20 -0400 Subject: [PATCH] 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: I3ec4ae79db525640bb5d6369727985d5071b7fd4 --- HACKING.rst | 1 + nova/virt/zun/driver.py | 30 ++++++++++++++---------------- nova/virt/zun/network.py | 2 +- nova/virt/zun/opencontrail.py | 4 ++-- nova/virt/zun/vifs.py | 4 ++-- zun/hacking/checks.py | 23 ++++++++++++++++++++--- zun/network/kuryr_network.py | 2 +- zun/tests/unit/test_hacking.py | 8 ++++++++ 8 files changed, 49 insertions(+), 25 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 4bc381f4b..f5558235c 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -20,3 +20,4 @@ Zun Specific Commandments - [Z338] Use assertIn/NotIn(A, B) rather than assertEqual(A in B, True/False). - [Z339] Don't use xrange() - [Z352] LOG.warn is deprecated. Enforce use of LOG.warning. +- [Z353] Don't translate logs. diff --git a/nova/virt/zun/driver.py b/nova/virt/zun/driver.py index 746b599c5..a7c97dc95 100644 --- a/nova/virt/zun/driver.py +++ b/nova/virt/zun/driver.py @@ -300,10 +300,9 @@ class DockerDriver(driver.ComputeDriver): if not hasattr(self, '_nodename'): self._nodename = nodename if nodename != self._nodename: - LOG.error(_('Hostname has changed from %(old)s to %(new)s. ' - 'A restart is required to take effect.' - ), {'old': self._nodename, - 'new': nodename}) + LOG.error('Hostname has changed from %(old)s to %(new)s. ' + 'A restart is required to take effect.', + {'old': self._nodename, 'new': nodename}) memory = hostinfo.get_memory_usage() disk = hostinfo.get_disk_usage() @@ -383,8 +382,8 @@ class DockerDriver(driver.ComputeDriver): except Exception as e: # If failed to load image from shared_directory, continue # to download the image from glance then load. - LOG.warning(_('Cannot load repository file from shared ' - 'directory: %s'), + LOG.warning('Cannot load repository file from shared ' + 'directory: %s', e, instance=instance, exc_info=True) # TODO(imain): It would be nice to do this with file like object @@ -408,7 +407,7 @@ class DockerDriver(driver.ComputeDriver): return self.docker.inspect_image( self._encode_utf8(image_meta.name)) except Exception as e: - LOG.warning(_('Cannot load repository file: %s'), + LOG.warning('Cannot load repository file: %s', e, instance=instance, exc_info=True) msg = _('Cannot load repository file: {0}') raise exception.NovaException(msg.format(e), @@ -478,7 +477,7 @@ class DockerDriver(driver.ComputeDriver): 'Timeout waiting for vif plugging', instance_id=instance['name']) except (Exception) as e: - LOG.warning(_('Cannot setup network: %s'), + LOG.warning('Cannot setup network: %s', e, instance=instance, exc_info=True) msg = _('Cannot setup network: {0}') self.docker.kill(container_id) @@ -580,7 +579,7 @@ class DockerDriver(driver.ComputeDriver): self.docker.stop(container_id, max(timeout, 5)) except errors.APIError as e: if 'Unpause the container before stopping' not in e.explanation: - LOG.warning(_('Cannot stop container: %s'), + LOG.warning('Cannot stop container: %s', e, instance=instance, exc_info=True) raise self.docker.unpause(container_id) @@ -623,9 +622,8 @@ class DockerDriver(driver.ComputeDriver): if network_info: self.unplug_vifs(instance, network_info) except Exception as e: - LOG.warning(_('Cannot destroy the container network' - ' during reboot {0}').format(e), - exc_info=True) + LOG.warning('Cannot destroy the container network' + ' during reboot {0}'.format(e), exc_info=True) return self.docker.start(container_id) @@ -634,7 +632,7 @@ class DockerDriver(driver.ComputeDriver): self.plug_vifs(instance, network_info) self._attach_vifs(instance, network_info) except Exception as e: - LOG.warning(_('Cannot setup network on reboot: {0}'), e, + LOG.warning('Cannot setup network on reboot: {0}', e, exc_info=True) return @@ -650,7 +648,7 @@ class DockerDriver(driver.ComputeDriver): self.plug_vifs(instance, network_info) self._attach_vifs(instance, network_info) except Exception as e: - LOG.debug(_('Cannot setup network: %s'), + LOG.debug('Cannot setup network: %s', e, instance=instance, exc_info=True) msg = _('Cannot setup network: {0}') self.docker.kill(container_id) @@ -674,7 +672,7 @@ class DockerDriver(driver.ComputeDriver): if not self.docker.pause(cont_id): raise exception.NovaException except Exception as e: - LOG.debug(_('Error pause container: %s'), + LOG.debug('Error pause container: %s', e, instance=instance, exc_info=True) msg = _('Cannot pause container: {0}') raise exception.NovaException(msg.format(e), @@ -690,7 +688,7 @@ class DockerDriver(driver.ComputeDriver): if not self.docker.unpause(cont_id): raise exception.NovaException except Exception as e: - LOG.debug(_('Error unpause container: %s'), + LOG.debug('Error unpause container: %s', e, instance=instance, exc_info=True) msg = _('Cannot unpause container: {0}') raise exception.NovaException(msg.format(e), diff --git a/nova/virt/zun/network.py b/nova/virt/zun/network.py index 6f77435dd..eb2745bee 100644 --- a/nova/virt/zun/network.py +++ b/nova/virt/zun/network.py @@ -33,7 +33,7 @@ def teardown_network(container_id): run_as_root=True) break except processutils.ProcessExecutionError: - LOG.warning(_('Cannot remove network namespace, netns id: %s'), + LOG.warning('Cannot remove network namespace, netns id: %s', container_id) diff --git a/nova/virt/zun/opencontrail.py b/nova/virt/zun/opencontrail.py index 4dbcdab0c..4865d388d 100644 --- a/nova/virt/zun/opencontrail.py +++ b/nova/virt/zun/opencontrail.py @@ -141,7 +141,7 @@ class OpenContrailVIFDriver(object): utils.execute('ip', 'netns', 'exec', container_id, 'ip', 'link', 'set', if_remote_name, 'up', run_as_root=True) except Exception: - LOG.exception(_("Failed to attach vif"), instance=instance) + LOG.exception("Failed to attach vif", instance=instance) def _retrieve_ip_address(self, vif, version): address = None @@ -174,4 +174,4 @@ class OpenContrailVIFDriver(object): utils.execute('ip', 'link', 'delete', if_local_name, run_as_root=True) except Exception: - LOG.exception(_("Delete port failed"), instance=instance) + LOG.exception("Delete port failed", instance=instance) diff --git a/nova/virt/zun/vifs.py b/nova/virt/zun/vifs.py index 33da8c86e..871458b9e 100644 --- a/nova/virt/zun/vifs.py +++ b/nova/virt/zun/vifs.py @@ -209,7 +209,7 @@ class DockerGenericVIFDriver(object): run_as_root=True) linux_net.delete_net_dev(if_local_name) except processutils.ProcessExecutionError: - LOG.exception(_("Failed while unplugging vif"), instance=instance) + LOG.exception("Failed while unplugging vif", instance=instance) def unplug_midonet(self, instance, vif): """Unplug into MidoNet's network port @@ -220,7 +220,7 @@ class DockerGenericVIFDriver(object): utils.execute('mm-ctl', '--unbind-port', vif['id'], run_as_root=True) except processutils.ProcessExecutionError: - LOG.exception(_("Failed while unplugging vif"), instance=instance) + LOG.exception("Failed while unplugging vif", instance=instance) def _create_veth_pair(self, if_local_name, if_remote_name, bridge): undo_mgr = utils.UndoManager() diff --git a/zun/hacking/checks.py b/zun/hacking/checks.py index feed41b0c..96009cc6f 100644 --- a/zun/hacking/checks.py +++ b/zun/hacking/checks.py @@ -48,6 +48,10 @@ dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") assert_xrange_re = re.compile( r"\s*xrange\s*\(") +log_levels = {"debug", "error", "info", "warning", "critical", "exception"} +translated_log = re.compile(r"(.)*LOG\.(%(levels)s)\(\s*_\(" % + {'levels': '|'.join(log_levels)}) + def no_mutable_default_args(logical_line): msg = "Z322: Method's default argument shouldn't be mutable!" @@ -121,8 +125,7 @@ def use_timeutils_utcnow(logical_line, filename): def dict_constructor_with_list_copy(logical_line): msg = ("Z336: Must use a dict comprehension instead of a dict constructor" - " with a sequence of key-value pairs." - ) + " with a sequence of key-value pairs.") if dict_constructor_with_list_copy_re.match(logical_line): yield (0, msg) @@ -136,11 +139,24 @@ def no_log_warn(logical_line): Z352 """ - msg = ("Z352: LOG.warn is deprecated, please use LOG.warning!") + msg = "Z352: LOG.warn is deprecated, please use LOG.warning!" if "LOG.warn(" in logical_line: yield (0, msg) +def no_translate_logs(logical_line): + """Check for 'LOG.*(_(' + + Starting with the Pike series, OpenStack no longer supports log + translation. + + Z353 + """ + msg = "Z353: Log messages should not be translated!" + if translated_log.match(logical_line): + yield (0, msg) + + def factory(register): register(no_mutable_default_args) register(assert_equal_true_or_false) @@ -151,3 +167,4 @@ def factory(register): register(dict_constructor_with_list_copy) register(no_xrange) register(no_log_warn) + register(no_translate_logs) diff --git a/zun/network/kuryr_network.py b/zun/network/kuryr_network.py index f4f730281..c1721add7 100644 --- a/zun/network/kuryr_network.py +++ b/zun/network/kuryr_network.py @@ -200,4 +200,4 @@ class KuryrNetwork(network.Network): {'port': updated_port}) except Exception: with excutils.save_and_reraise_exception(): - LOG.exception(_("Neutron Error:")) + LOG.exception("Neutron Error:") diff --git a/zun/tests/unit/test_hacking.py b/zun/tests/unit/test_hacking.py index c8adae2f5..6ccb958c9 100644 --- a/zun/tests/unit/test_hacking.py +++ b/zun/tests/unit/test_hacking.py @@ -232,3 +232,11 @@ class HackingTestCase(base.BaseTestCase): LOG.warning("LOG.warn is deprecated") """ self._assert_has_no_errors(code, check) + + def test_no_log_translations(self): + for log in checks.log_levels: + bad = 'LOG.%s(_("Bad"))' % log + self.assertEqual(1, len(list(checks.no_translate_logs(bad)))) + # Catch abuses when used with a variable and not a literal + bad = 'LOG.%s(_(msg))' % log + self.assertEqual(1, len(list(checks.no_translate_logs(bad))))