From a3d7d73a69f85b396aad30610294d5ea246a0df7 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Sat, 28 Mar 2020 12:00:14 +0100 Subject: [PATCH] Bump hacking to 3.0.0 The new version enables a lot of standard flake8 checks. Some of them are temporary disabled to reduce the scope of this patch: * Complexity check requires a few functions to be rewritten (apparently, it was not enabled previously). * Indentation check failures are numerous and potentially contradictive. These checks will be enabled in follow-ups. W606 is removed from excludes since we no longer hit it. Change-Id: I1e5a6f8e5e90c55cfc6f740b26c30196512d3be3 --- doc/source/conf.py | 1 + ironic/cmd/status.py | 1 + ironic/common/pxe_utils.py | 2 +- ironic/conductor/manager.py | 2 +- ...cc9_add_extra_column_to_deploy_templates.py | 6 +++--- .../cd2c80feb331_add_node_retired_field.py | 6 +++--- ironic/drivers/base.py | 2 +- ironic/drivers/modules/snmp.py | 5 +++-- ironic/hacking/checks.py | 7 +++---- .../api/controllers/v1/test_deploy_template.py | 2 +- .../unit/drivers/modules/test_deploy_utils.py | 4 ++-- lower-constraints.txt | 2 +- test-requirements.txt | 2 +- tox.ini | 18 +++++++++++------- 14 files changed, 33 insertions(+), 27 deletions(-) diff --git a/doc/source/conf.py b/doc/source/conf.py index 442f2c208a..02223e4bd8 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -54,6 +54,7 @@ apidoc_excluded_paths = [ 'db/sqlalchemy/alembic/env', 'db/sqlalchemy/alembic/versions/*', 'drivers/modules/ansible/playbooks*', + 'hacking', 'tests', ] apidoc_separate_modules = True diff --git a/ironic/cmd/status.py b/ironic/cmd/status.py index a1a1b5502c..e7f10fb728 100644 --- a/ironic/cmd/status.py +++ b/ironic/cmd/status.py @@ -61,5 +61,6 @@ def main(): return upgradecheck.main( cfg.CONF, project='ironic', upgrade_command=Checks()) + if __name__ == '__main__': sys.exit(main()) diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index 5088a872bd..1595856689 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -809,7 +809,7 @@ def get_volume_pxe_options(task): def __get_property(properties, key): prop = __return_item_or_first_if_list(properties.get(key, '')) - if prop is not '': + if prop != '': return prop return __return_item_or_first_if_list(properties.get(key + 's', '')) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 2e435cf684..3f595c3f1a 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -3657,7 +3657,7 @@ def do_sync_power_state(task, count): # node_power_action will update the node record # so don't do that again here. utils.node_power_action(task, node.power_state) - except Exception as e: + except Exception: LOG.error( "Failed to change power state of node %(node)s " "to '%(state)s', attempt %(attempt)s of %(retries)s.", diff --git a/ironic/db/sqlalchemy/alembic/versions/1e15e7122cc9_add_extra_column_to_deploy_templates.py b/ironic/db/sqlalchemy/alembic/versions/1e15e7122cc9_add_extra_column_to_deploy_templates.py index 854b9c2e7a..849b170694 100644 --- a/ironic/db/sqlalchemy/alembic/versions/1e15e7122cc9_add_extra_column_to_deploy_templates.py +++ b/ironic/db/sqlalchemy/alembic/versions/1e15e7122cc9_add_extra_column_to_deploy_templates.py @@ -18,13 +18,13 @@ Create Date: 2019-02-26 15:08:18.419157 """ +from alembic import op +import sqlalchemy as sa + # revision identifiers, used by Alembic. revision = '1e15e7122cc9' down_revision = '2aac7e0872f6' -from alembic import op -import sqlalchemy as sa - def upgrade(): op.add_column('deploy_templates', diff --git a/ironic/db/sqlalchemy/alembic/versions/cd2c80feb331_add_node_retired_field.py b/ironic/db/sqlalchemy/alembic/versions/cd2c80feb331_add_node_retired_field.py index 027e7659d9..5bfa31e78a 100644 --- a/ironic/db/sqlalchemy/alembic/versions/cd2c80feb331_add_node_retired_field.py +++ b/ironic/db/sqlalchemy/alembic/versions/cd2c80feb331_add_node_retired_field.py @@ -18,13 +18,13 @@ Create Date: 2020-01-16 12:51:13.866882 """ +from alembic import op +import sqlalchemy as sa + # revision identifiers, used by Alembic. revision = 'cd2c80feb331' down_revision = 'ce6c4b3cf5a2' -from alembic import op -import sqlalchemy as sa - def upgrade(): op.add_column('nodes', sa.Column('retired', sa.Boolean(), nullable=True, diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index a407cf5363..525342a6e4 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -750,7 +750,7 @@ def _passthru(http_methods, method=None, async_call=True, def passthru_handler(*args, **kwargs): try: return func(*args, **kwargs) - except exception.IronicException as e: + except exception.IronicException: with excutils.save_and_reraise_exception(): LOG.exception(passthru_logmessage, api_method) except Exception as e: diff --git a/ironic/drivers/modules/snmp.py b/ironic/drivers/modules/snmp.py index 2a21550505..462053bf1d 100644 --- a/ironic/drivers/modules/snmp.py +++ b/ironic/drivers/modules/snmp.py @@ -404,7 +404,7 @@ def memoize(f): def memoized(self, node_info): hashable_node_info = frozenset((key, val) for key, val in node_info.items() - if key is not 'outlet') + if key != 'outlet') if hashable_node_info not in _memoized: _memoized[hashable_node_info] = f(self) return _memoized[hashable_node_info] @@ -420,7 +420,7 @@ def retry_on_outdated_cache(f): hashable_node_info = ( frozenset((key, val) for key, val in self.snmp_info.items() - if key is not 'outlet') + if key != 'outlet') ) del _memoized[hashable_node_info] self.driver = self._get_pdu_driver(self.snmp_info) @@ -858,6 +858,7 @@ class SNMPDriverAuto(SNMPDriverBase): def _fetch_driver(self): return self.client.get(self.SYS_OBJ_OID) + # A dictionary of supported drivers keyed by snmp_driver attribute DRIVER_CLASSES = { 'apc': SNMPDriverAPCMasterSwitch, diff --git a/ironic/hacking/checks.py b/ironic/hacking/checks.py index 6c2ac23fd0..1d59efb372 100644 --- a/ironic/hacking/checks.py +++ b/ironic/hacking/checks.py @@ -14,6 +14,8 @@ import re +from hacking import core + # N323: Found use of _() without explicit import of _! @@ -29,6 +31,7 @@ underscore_import_check = re.compile(r"(.)*import _(.)*") custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*") +@core.flake8ext def check_explicit_underscore_import(logical_line, filename): """Check for explicit import of the _ function @@ -49,7 +52,3 @@ def check_explicit_underscore_import(logical_line, filename): elif (translated_log.match(logical_line) or string_translation.match(logical_line)): yield(0, "N323: Found use of _() without explicit import of _!") - - -def factory(register): - register(check_explicit_underscore_import) diff --git a/ironic/tests/unit/api/controllers/v1/test_deploy_template.py b/ironic/tests/unit/api/controllers/v1/test_deploy_template.py index f45ec138d7..6d7dfcb35d 100644 --- a/ironic/tests/unit/api/controllers/v1/test_deploy_template.py +++ b/ironic/tests/unit/api/controllers/v1/test_deploy_template.py @@ -655,7 +655,7 @@ class TestPatch(BaseDeployTemplatesAPITest): def test_add_root_non_existent(self, mock_save): patch = [{'path': '/foo', 'value': 'bar', 'op': 'add'}] self._test_update_bad_request( - mock_save, patch, "Adding a new attribute \(/foo\)") + mock_save, patch, "Adding a new attribute \\(/foo\\)") def test_add_too_high_index_step_fail(self, mock_save): step = { diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index be2472c890..20e578bfb2 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -938,8 +938,8 @@ class ParseInstanceInfoCapabilitiesTestCase(tests_base.TestCase): result = boot_mode_utils.get_boot_mode_for_deploy(self.node) self.assertEqual('bios', result) - instance_info = {'capabilities': {'trusted_boot': 'True'}, - 'capabilities': {'secure_boot': 'True'}} + instance_info = {'capabilities': {'trusted_boot': 'True', + 'secure_boot': 'True'}} self.node.instance_info = instance_info result = boot_mode_utils.get_boot_mode_for_deploy(self.node) diff --git a/lower-constraints.txt b/lower-constraints.txt index 8468a14303..75b60e8dc3 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -10,7 +10,7 @@ eventlet==0.18.2 fixtures==3.0.0 flake8-import-order==0.13 futurist==1.2.0 -hacking==1.0.0 +hacking==3.0.0 ironic-lib==2.17.1 iso8601==0.1.11 Jinja2==2.10 diff --git a/test-requirements.txt b/test-requirements.txt index ab53cb70f6..26e52b3fce 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,7 @@ # The order of packages is significant, because pip processes them in the order # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -hacking>=1.0.0,<1.1.0 # Apache-2.0 +hacking>=3.0.0,<3.1.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0 ddt>=1.0.1 # MIT doc8>=0.6.0 # Apache-2.0 diff --git a/tox.ini b/tox.ini index 9ce4b03a37..984742a26c 100644 --- a/tox.ini +++ b/tox.ini @@ -111,27 +111,31 @@ commands = {posargs} [flake8] # [W503] Line break before binary operator. -# NOTE(TheJulia): Adding W606 to the ignore list -# until we are able to remove them due to deprecation -# period passing. Patch merged in March 2018 to rename -# method arguments from reserved keywords. -ignore = E129,W503,W606 +# FIXME(dtantsur): W504 and E117 disabled temporary due to a lot of hits +ignore = E117,E129,W503,W504 filename = *.py,app.wsgi exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build import-order-style = pep8 application-import-names = ironic -max-complexity=18 +# FIXME(dtantsur): uncomment and fix the code +#max-complexity=18 # [H106] Don't put vim configuration in source files. # [H203] Use assertIs(Not)None to check for None. # [H204] Use assert(Not)Equal to check for equality. # [H205] Use assert(Greater|Less)(Equal) for comparison. +# TODO(dtantsur): [H210] Require ‘autospec’, ‘spec’, or ‘spec_set’ in mock.patch/mock.patch.object calls # [H904] Delay string interpolations at logging calls. enable-extensions=H106,H203,H204,H205,H904 [hacking] -local-check-factory = ironic.hacking.checks.factory import_exceptions = testtools.matchers, ironic.common.i18n +[flake8:local-plugins] +# [N323] Found use of _() without explicit import of _! +extension = + N323 = checks:check_explicit_underscore_import +paths = ./ironic/hacking/ + [testenv:lower-constraints] deps = -c{toxinidir}/lower-constraints.txt