From c61976b66dac644a2be3c6b6dd62e343156a5fd8 Mon Sep 17 00:00:00 2001 From: Boden R Date: Wed, 7 Dec 2016 14:32:56 -0700 Subject: [PATCH] Use new checks in hacking 0.12 Neutron recently updated their usage of hacking to use version 0.12 [1] that now contains the hacking check check_delayed_string_interpolation who's off_by_default value is initially True (i.e. disabled). This hacking check is defined in neutron_lib, but hasn't been registered in our factory and isn't being consumed directly [2]. This patch takes a simple approach to reusing openstack-dev hacking checks: - Removes all traces of neutron-lib's version of the check; no one is using it [2]. - Bumps our version of hacking to use 0.12 so we can use the checks in that release. - Enables the check via enabled extensions in tox.ini as neutron did [1]. - Updates our hacking check usage, noting that adopters should enable the same extensions we do (via tox.ini). [1] https://review.openstack.org/#/c/394817/ [2] http://codesearch.openstack.org/?q=check_delayed_string_interpolation&i=nope&files=&repos= Change-Id: Ie9448317855b9cba6092cd0f63b77d26a562a5c9 --- HACKING.rst | 1 - doc/source/usage.rst | 13 +++++++ neutron_lib/hacking/checks.py | 1 - neutron_lib/hacking/translation_checks.py | 30 ---------------- neutron_lib/tests/unit/hacking/test_checks.py | 34 ------------------- ...e-hacking-check-H904-f512ecc98c0a4033.yaml | 5 +++ test-requirements.txt | 2 +- tox.ini | 2 ++ 8 files changed, 21 insertions(+), 67 deletions(-) create mode 100644 releasenotes/notes/enable-hacking-check-H904-f512ecc98c0a4033.yaml diff --git a/HACKING.rst b/HACKING.rst index 5fc3a9575..42f3a76fa 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -21,4 +21,3 @@ Neutron Library Specific Commandments - [N533] Validate that debug level logs are not translated - [N534] Exception messages should be translated - [N535] Usage of Python eventlet module not allowed -- [N536] String interpolation should be delayed at logging calls. diff --git a/doc/source/usage.rst b/doc/source/usage.rst index 9498d7d08..1f3d9ff6d 100644 --- a/doc/source/usage.rst +++ b/doc/source/usage.rst @@ -41,6 +41,19 @@ To adopt neutron-lib's hacking checks: [hacking] local-check-factory = myproject.mypkg.my_factory +#. Update your project's ``tox.ini`` enable any flake8 extensions neutron-lib's + ``tox.ini`` does. These are hacking checks otherwise disabled by default + that neutron-lib expects to run. + + For example in neutron-lib's ``tox.ini``:: + + [flake8] + # H904: Delay string interpolations at logging calls + enable-extensions=H904 + + In the example above, adopters should also add ``H904`` to the + ``enable-extensions`` in their ``tox.ini``. + #. Actively adopt neutron-lib hacking checks that are incubating and will soon become adopter checks by manually running the checks on your project. This can be done by modifying your ``tox.ini`` to use the diff --git a/neutron_lib/hacking/checks.py b/neutron_lib/hacking/checks.py index 6d6d0b962..f37a6fb17 100644 --- a/neutron_lib/hacking/checks.py +++ b/neutron_lib/hacking/checks.py @@ -255,7 +255,6 @@ def incubating_factory(register): :param register: The function to register the check functions with. :returns: None. """ - register(translation_checks.check_delayed_string_interpolation) def _neutron_lib_factory(register): diff --git a/neutron_lib/hacking/translation_checks.py b/neutron_lib/hacking/translation_checks.py index db45505d1..3831ec3d4 100644 --- a/neutron_lib/hacking/translation_checks.py +++ b/neutron_lib/hacking/translation_checks.py @@ -41,8 +41,6 @@ def _regex_for_level(level, hint): _log_translation_hint = re.compile( '|'.join('(?:%s)' % _regex_for_level(level, hint) for level, hint in _all_log_levels.items())) -_log_string_interpolation = re.compile( - r".*LOG\.(error|warning|info|critical|exception|debug)\([^,]*%[^,]*[,)]") def _translation_is_not_expected(filename): @@ -127,31 +125,3 @@ def check_raised_localized_exceptions(logical_line, filename): if exception_msg.startswith("\"") or exception_msg.startswith("\'"): msg = "N534: Untranslated exception message." yield (logical_line.index(exception_msg), msg) - - -def check_delayed_string_interpolation(logical_line, filename, noqa): - """N536 - String interpolation should be delayed at logging calls. - - N536: LOG.debug('Example: %s' % 'bad') - Okay: LOG.debug('Example: %s', 'good') - - :param logical_line: The logical line to check. - :param filename: The file name where the logical line exists. - :param noqa: Noqa indicator. - :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. - """ - msg = ("N536 String interpolation should be delayed to be " - "handled by the logging code, rather than being done " - "at the point of the logging call. " - "Use ',' instead of '%'.") - - if noqa: - return - - if '/tests/' in filename: - return - - if _log_string_interpolation.match(logical_line): - yield(logical_line.index('%'), msg) diff --git a/neutron_lib/tests/unit/hacking/test_checks.py b/neutron_lib/tests/unit/hacking/test_checks.py index 5dc68ff88..283b226f7 100644 --- a/neutron_lib/tests/unit/hacking/test_checks.py +++ b/neutron_lib/tests/unit/hacking/test_checks.py @@ -10,8 +10,6 @@ # License for the specific language governing permissions and limitations # under the License. -import re - import testtools from neutron_lib.hacking import checks @@ -213,38 +211,6 @@ class HackingTestCase(base.BaseTestCase): self.assertLinePasses(f, "raise KeyError('Error text')", 'neutron_lib/tests/unit/mytest.py') - def test_check_delayed_string_interpolation(self): - dummy_noqa = re.search('a', 'a') - f = tc.check_delayed_string_interpolation - - # In 'logical_line', Contents of strings replaced with - # "xxx" of same length. - self.assertLineFails(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)', - 'neutron_lib/db/utils.py', None) - self.assertLineFails(f, "LOG.warning(msg % 'xxxxx')", - 'neutron_lib/db/utils.py', None) - self.assertLineFails(f, "LOG.info(_LI('xxxxxxxxxxxxxxxxxxxxxxxxx" - "xxxxxxxxx') % {'xx': v1, 'xx': v2})", - 'neutron_lib/db/utils.py', None) - - self.assertLinePasses( - f, 'LOG.error(_LE("xxxxxxxxxxxxxxxxxx"), value)', - 'neutron_lib/db/utils.py', None) - self.assertLinePasses(f, "LOG.warning(msg, 'xxxxx')", - 'neutron_lib/db/utils.py', None) - self.assertLinePasses(f, "LOG.info(_LI('xxxxxxxxxxxxxxxxxxxxxxxx" - "xxxxxxxxx'), {'xx': v1, 'xx': v2})", - 'neutron_lib/db/utils.py', None) - - # check a file in neutron_lib/tests - self.assertLinePasses(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)', - 'neutron_lib/tests/unit/test_neutron_lib.py', - None) - # check code including 'noqa' - self.assertLinePasses(f, 'LOG.error(_LE("xxxxxxxxxxxxxxx") % value)', - 'neutron_lib/db/utils.py', - dummy_noqa) - def test_check_eventlet_imports(self): f = checks.check_no_eventlet_imports self.assertLineFails(f, "import eventlet") diff --git a/releasenotes/notes/enable-hacking-check-H904-f512ecc98c0a4033.yaml b/releasenotes/notes/enable-hacking-check-H904-f512ecc98c0a4033.yaml new file mode 100644 index 000000000..7c42f2907 --- /dev/null +++ b/releasenotes/notes/enable-hacking-check-H904-f512ecc98c0a4033.yaml @@ -0,0 +1,5 @@ +--- +other: + - OpenStack dev hacking check ``H904`` is now enabled in ``tox.ini`` + via the ``enable-extensions`` configuration property. Neutron-lib + adopters should also enable this hacking check in their ``tox.ini``. diff --git a/test-requirements.txt b/test-requirements.txt index fbbf5ca49..8756bfdf8 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,7 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -hacking<0.12,>=0.11.0 # Apache-2.0 +hacking>=0.12.0,!=0.13.0,<0.14 # Apache-2.0 coverage>=4.0 # Apache-2.0 discover # BSD diff --git a/tox.ini b/tox.ini index 1db6c9f6f..8888b0ea6 100644 --- a/tox.ini +++ b/tox.ini @@ -54,6 +54,8 @@ commands = {toxinidir}/tools/api_report.sh [flake8] +# H904: Delay string interpolations at logging calls +enable-extensions=H904 show-source = True exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build,tools