openstack/ironic
Code Issues Proposed changes

Bump hacking to 3.0.0

Browse Source 
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
This commit is contained in:
Dmitry Tantsur 2020-03-28 12:00:14 +01:00 committed by Riccardo Pittau
parent 4829df2966
commit a3d7d73a69
14 changed files with 33 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
doc/source/conf.py
View File

@ -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

1
ironic/cmd/status.py
View File

@ -61,5 +61,6 @@ def main():
return upgradecheck.main(
cfg.CONF, project='ironic', upgrade_command=Checks())
if __name__ == '__main__':
sys.exit(main())

2
ironic/common/pxe_utils.py
View File

@ -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', ''))

2
ironic/conductor/manager.py
View File

@ -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.",

6
ironic/db/sqlalchemy/alembic/versions/1e15e7122cc9_add_extra_column_to_deploy_templates.py
View File

@ -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',

6
ironic/db/sqlalchemy/alembic/versions/cd2c80feb331_add_node_retired_field.py
View File

@ -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,

2
ironic/drivers/base.py
View File

@ -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:

5
ironic/drivers/modules/snmp.py
View File

@ -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,

7
ironic/hacking/checks.py
View File

@ -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)

2
ironic/tests/unit/api/controllers/v1/test_deploy_template.py
View File

@ -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 = {

4
ironic/tests/unit/drivers/modules/test_deploy_utils.py
View File

@ -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)

2
lower-constraints.txt
View File

@ -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

2
test-requirements.txt
View File

@ -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

18
tox.ini
View File

@ -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