summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorpoojajadhav <pooja.jadhav@nttdata.com>2017-04-04 18:42:01 +0530
committerpooja jadhav <pooja.jadhav@nttdata.com>2017-09-25 14:34:25 +0530
commit892e9b116c6a9d1197582f72efe4011dea43a242 (patch)
tree88a3a585fe9034cb300d954d25d84ddfaea98a2b
parent12f91c29a79f765aa5d6ec3da4e5eead28f7ef50 (diff)
Hacking: do not translate log messages
Community has decided to do not translate log messages anymore. The motivation behind removing translation jobs for log messages is that when operators encounters any unfamiliar situations(e.g. nova error, keystone error etc.), searching on the Internet based on actual log content would have more results than searching translated log messages. In this patch I have added hacking rule to check that logs are not translated. Change-Id: I90d53d617aac6839c2f2ab119847cbf24a1300e0
Notes
Notes (review): Verified+1: Jenkins Code-Review+2: Tushar Patil <tushar.vitthal.patil@gmail.com> Code-Review+2: Sampath Priyankara (samP) <sam47priya@gmail.com> Workflow+1: Sampath Priyankara (samP) <sam47priya@gmail.com> Code-Review+1: yanpeifei <yanpeifei@gohighsec.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Fri, 29 Sep 2017 00:45:53 +0000 Reviewed-on: https://review.openstack.org/454740 Project: openstack/masakari Branch: refs/heads/master
-rw-r--r--HACKING.rst3
-rw-r--r--masakari/hacking/checks.py26
-rw-r--r--masakari/tests/unit/test_hacking.py24
3 files changed, 31 insertions, 22 deletions
diff --git a/HACKING.rst b/HACKING.rst
index cb250b5..fcb481f 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -19,11 +19,10 @@ Masakari Specific Commandments
19 assertIsInstance(A, B). 19 assertIsInstance(A, B).
20- [M306] Change assertEqual(type(A), B) by optimal assert like 20- [M306] Change assertEqual(type(A), B) by optimal assert like
21 assertIsInstance(A, B) 21 assertIsInstance(A, B)
22- [M308] Validate that debug level logs are not translated. 22- [M308] Validate that log messages are not translated.
23- [M309] Don't import translation in tests 23- [M309] Don't import translation in tests
24- [M310] Setting CONF.* attributes directly in tests is forbidden. Use 24- [M310] Setting CONF.* attributes directly in tests is forbidden. Use
25 self.flags(option=value) instead. 25 self.flags(option=value) instead.
26- [M314] Log messages require translations!
27- [M315] Method's default argument shouldn't be mutable 26- [M315] Method's default argument shouldn't be mutable
28- [M316] Ensure that the _() function is explicitly imported to ensure proper translations. 27- [M316] Ensure that the _() function is explicitly imported to ensure proper translations.
29- [M317] Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s 28- [M317] Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s
diff --git a/masakari/hacking/checks.py b/masakari/hacking/checks.py
index baaac04..9617cd1 100644
--- a/masakari/hacking/checks.py
+++ b/masakari/hacking/checks.py
@@ -85,6 +85,15 @@ spawn_re = re.compile(
85contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(") 85contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(")
86doubled_words_re = re.compile( 86doubled_words_re = re.compile(
87 r"\b(then?|[iao]n|i[fst]|but|f?or|at|and|[dt]o)\s+\1\b") 87 r"\b(then?|[iao]n|i[fst]|but|f?or|at|and|[dt]o)\s+\1\b")
88_all_log_levels = {'critical', 'error', 'exception', 'info',
89 'warning', 'debug'}
90_all_hints = {'_', '_LE', '_LI', '_LW', '_LC'}
91
92log_translation_re = re.compile(
93 r".*LOG\.(%(levels)s)\(\s*(%(hints)s)\(" % {
94 'levels': '|'.join(_all_log_levels),
95 'hints': '|'.join(_all_hints),
96 })
88 97
89 98
90def no_db_session_in_public_api(logical_line, filename): 99def no_db_session_in_public_api(logical_line, filename):
@@ -139,21 +148,18 @@ def assert_equal_type(logical_line):
139 yield (0, "M306: assertEqual(type(A), B) sentences not allowed") 148 yield (0, "M306: assertEqual(type(A), B) sentences not allowed")
140 149
141 150
142def no_translate_debug_logs(logical_line, filename): 151def no_translate_logs(logical_line):
143 """Check for 'LOG.debug(_(' 152 """Check for 'LOG.*(_*("'
144 153
145 As per our translation policy, 154 OpenStack no longer supports log translation, so we shouldn't
146 https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation 155 translate logs.
147 we shouldn't translate debug level logs.
148 156
149 * This check assumes that 'LOG' is a logger. 157 * This check assumes that 'LOG' is a logger.
150 * Use filename so we can start enforcing this in specific folders instead
151 of needing to do so all at once.
152 158
153 M308 159 M308
154 """ 160 """
155 if logical_line.startswith("LOG.debug(_("): 161 if log_translation_re.match(logical_line):
156 yield(0, "M308 Don't translate debug level logs") 162 yield(0, "M308: Log messages should not be translated")
157 163
158 164
159def no_import_translation_in_tests(logical_line, filename): 165def no_import_translation_in_tests(logical_line, filename):
@@ -384,7 +390,7 @@ def factory(register):
384 register(assert_true_instance) 390 register(assert_true_instance)
385 register(assert_equal_type) 391 register(assert_equal_type)
386 register(assert_raises_regexp) 392 register(assert_raises_regexp)
387 register(no_translate_debug_logs) 393 register(no_translate_logs)
388 register(no_setting_conf_directly_in_tests) 394 register(no_setting_conf_directly_in_tests)
389 register(no_mutable_default_args) 395 register(no_mutable_default_args)
390 register(check_explicit_underscore_import) 396 register(check_explicit_underscore_import)
diff --git a/masakari/tests/unit/test_hacking.py b/masakari/tests/unit/test_hacking.py
index de52b97..6852a69 100644
--- a/masakari/tests/unit/test_hacking.py
+++ b/masakari/tests/unit/test_hacking.py
@@ -14,6 +14,7 @@
14 14
15import textwrap 15import textwrap
16 16
17import ddt
17import mock 18import mock
18import pep8 19import pep8
19 20
@@ -21,6 +22,7 @@ from masakari.hacking import checks
21from masakari import test 22from masakari import test
22 23
23 24
25@ddt.ddt
24class HackingTestCase(test.NoDBTestCase): 26class HackingTestCase(test.NoDBTestCase):
25 """This class tests the hacking checks in masakari.hacking.checks by 27 """This class tests the hacking checks in masakari.hacking.checks by
26 passing strings to the check methods like the pep8/flake8 parser would. 28 passing strings to the check methods like the pep8/flake8 parser would.
@@ -151,16 +153,6 @@ class HackingTestCase(test.NoDBTestCase):
151 self.assertEqual(len(list(checks.assert_true_or_false_with_in( 153 self.assertEqual(len(list(checks.assert_true_or_false_with_in(
152 "self.assertFalse(some in list1 and some2 in list2)"))), 0) 154 "self.assertFalse(some in list1 and some2 in list2)"))), 0)
153 155
154 def test_no_translate_debug_logs(self):
155 self.assertEqual(len(list(checks.no_translate_debug_logs(
156 "LOG.debug(_('foo'))", "masakari/foo.py"))), 1)
157
158 self.assertEqual(len(list(checks.no_translate_debug_logs(
159 "LOG.debug('foo')", "masakari/foo.py"))), 0)
160
161 self.assertEqual(len(list(checks.no_translate_debug_logs(
162 "LOG.info(_('foo'))", "masakari/foo.py"))), 0)
163
164 def test_no_setting_conf_directly_in_tests(self): 156 def test_no_setting_conf_directly_in_tests(self):
165 self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests( 157 self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests(
166 "CONF.option = 1", "masakari/tests/test_foo.py"))), 1) 158 "CONF.option = 1", "masakari/tests/test_foo.py"))), 1)
@@ -431,3 +423,15 @@ class HackingTestCase(test.NoDBTestCase):
431 LOG.warning("LOG.warn is deprecated") 423 LOG.warning("LOG.warn is deprecated")
432 """ 424 """
433 self._assert_has_no_errors(code, checks.no_log_warn) 425 self._assert_has_no_errors(code, checks.no_log_warn)
426
427 @ddt.data('LOG.info(_LI("Bad"))',
428 'LOG.warning(_LW("Bad"))',
429 'LOG.error(_LE("Bad"))',
430 'LOG.exception(_("Bad"))',
431 'LOG.debug(_("Bad"))',
432 'LOG.critical(_LC("Bad"))')
433 def test_no_translate_logs(self, log_statement):
434 self.assertEqual(1, len(list(checks.no_translate_logs(log_statement))))
435 errors = [(1, 0, 'M308')]
436 self._assert_has_errors(log_statement, checks.no_translate_logs,
437 expected_errors=errors)