From 0f9c00a08f074f41d875b5c4640fa20a69610be7 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Cuong Date: Tue, 4 Jul 2017 05:09:01 -0400 Subject: [PATCH] Update log translation hacking rule Starting with the Pike series, OpenStack no longer supports log translation. Update hacking rule to prevent log translation in all log level instead of only debug level. Since all log translations are invalidated, check_explicit_underscore_import can be simplified. Change-Id: If941dc7c1052cea6a3d011980776ee272f8d39a3 --- HACKING.rst | 6 +++--- cloudkitty/hacking/checks.py | 29 ++++++++++++++--------------- cloudkitty/tests/test_hacking.py | 16 +++++++--------- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index b9331dab..7a70a9cd 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -1,5 +1,5 @@ Cloudkitty Style Commandments -============================ +============================= - Step 1: Read the OpenStack Style Commandments http://docs.openstack.org/developer/hacking/ @@ -7,12 +7,12 @@ Cloudkitty Style Commandments Cloudkitty Specific Commandments -------------------------------- +-------------------------------- - [C310] Check for improper use of logging format arguments. - [C311] Use assertIsNone(...) instead of assertEqual(None, ...). - [C312] Use assertTrue(...) rather than assertEqual(True, ...). -- [C313] Validate that debug level logs are not translated. +- [C313] Validate that logs are not translated. - [C314] str() and unicode() cannot be used on an exception. Remove or use six.text_type(). - [C315] Translated messages cannot be concatenated. String should be included in translated message. diff --git a/cloudkitty/hacking/checks.py b/cloudkitty/hacking/checks.py index f13bb480..d5eada22 100644 --- a/cloudkitty/hacking/checks.py +++ b/cloudkitty/hacking/checks.py @@ -36,11 +36,12 @@ Guidelines for writing new hacking checks UNDERSCORE_IMPORT_FILES = [] -log_translation = re.compile( - r"(.)*LOG\.(audit|error|info|critical|exception)\(\s*('|\")") -translated_log = re.compile( - r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)" - "\(\s*_\(\s*('|\")") +_all_log_levels = {'debug', 'error', 'info', 'warning', + 'critical', 'exception'} +# Since _Lx have been removed, we just need to check _() +translated_logs = re.compile( + r"(.)*LOG\.(%(level)s)\(\s*_\(" % {'level': '|'.join(_all_log_levels)}) + string_translation = re.compile(r"[^_]*_\(\s*('|\")") underscore_import_check = re.compile(r"(.)*import _$") underscore_import_check_multi = re.compile(r"(.)*import (.)*_, (.)*") @@ -99,12 +100,11 @@ class BaseASTChecker(ast.NodeVisitor): return False -def no_translate_debug_logs(logical_line, filename): - """Check for 'LOG.debug(_(' +def no_translate_logs(logical_line, filename): + """Check for 'LOG.*(_(' - As per our translation policy, - https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation - we shouldn't translate debug level logs. + Starting with the Pike series, OpenStack no longer supports log + translation. * This check assumes that 'LOG' is a logger. * Use filename so we can start enforcing this in specific folders instead @@ -112,8 +112,8 @@ def no_translate_debug_logs(logical_line, filename): C313 """ - if logical_line.startswith("LOG.debug(_("): - yield(0, "C313 Don't translate debug level logs") + if translated_logs.match(logical_line): + yield(0, "C313 Don't translate logs") class CheckLoggingFormatArgs(BaseASTChecker): @@ -198,8 +198,7 @@ def check_explicit_underscore_import(logical_line, filename): underscore_import_check_multi.match(logical_line) or custom_underscore_check.match(logical_line)): UNDERSCORE_IMPORT_FILES.append(filename) - elif (translated_log.match(logical_line) or - string_translation.match(logical_line)): + elif string_translation.match(logical_line): yield(0, "C321: Found use of _() without explicit import of _ !") @@ -341,7 +340,7 @@ def no_log_warn_check(logical_line): def factory(register): register(check_explicit_underscore_import) - register(no_translate_debug_logs) + register(no_translate_logs) register(CheckForStrUnicodeExc) register(CheckLoggingFormatArgs) register(CheckForTransAdd) diff --git a/cloudkitty/tests/test_hacking.py b/cloudkitty/tests/test_hacking.py index 4a39ae64..ed45fb23 100644 --- a/cloudkitty/tests/test_hacking.py +++ b/cloudkitty/tests/test_hacking.py @@ -57,15 +57,13 @@ class HackingTestCase(tests.TestCase): should pass. """ - def test_no_translate_debug_logs(self): - self.assertEqual(1, len(list(checks.no_translate_debug_logs( - "LOG.debug(_('foo'))", "cloudkitty/scheduler/foo.py")))) - - self.assertEqual(0, len(list(checks.no_translate_debug_logs( - "LOG.debug('foo')", "cloudkitty/scheduler/foo.py")))) - - self.assertEqual(0, len(list(checks.no_translate_debug_logs( - "LOG.info(_('foo'))", "cloudkitty/scheduler/foo.py")))) + def test_no_log_translations(self): + for log in checks._all_log_levels: + bad = 'LOG.%s(_("Bad"))' % log + 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(_(msg))' % log + self.assertEqual(1, len(list(checks.no_translate_logs(bad, 'f')))) def test_check_explicit_underscore_import(self): self.assertEqual(1, len(list(checks.check_explicit_underscore_import(