diff --git a/HACKING.rst b/HACKING.rst index 18b63422c11..c2346e3c244 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -28,7 +28,6 @@ Below you can find a list of checks specific to this repository. - [N344] Python 3: Do not use filter(lambda obj: test(obj), data). Replace it with [obj for obj in data if test(obj)]. - [N346] Use neutron_lib.db.api.sqla_listen rather than sqlalchemy -- [N347] Test code must not import mock library - [N348] Test code must not import six library .. note:: diff --git a/neutron/__init__.py b/neutron/__init__.py index ae39f8be19a..445b900d378 100644 --- a/neutron/__init__.py +++ b/neutron/__init__.py @@ -16,12 +16,12 @@ import builtins import gettext -from debtcollector import removals +from neutron._i18n import _ as n_under gettext.install('neutron') -builtins.__dict__['_'] = removals.remove( - message='Builtin _ translation function is deprecated in OpenStack; ' - 'use the function from _i18n module for your project.')(_) # noqa +# gettext will install its own translation function, override it to be +# the one from neutron +builtins.__dict__['_'] = n_under diff --git a/neutron/db/agents_db.py b/neutron/db/agents_db.py index 99e5911dfaf..e282173684b 100644 --- a/neutron/db/agents_db.py +++ b/neutron/db/agents_db.py @@ -241,7 +241,7 @@ class AgentDbMixin(ext_agent.AgentPluginBase, AgentAvailabilityZoneMixin): configs = agent.get('configurations', {}) load_type = None load = 0 - if(agent['agent_type'] == constants.AGENT_TYPE_DHCP): + if (agent['agent_type'] == constants.AGENT_TYPE_DHCP): load_type = cfg.CONF.dhcp_load_type if load_type: load = int(configs.get(load_type, 0)) diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 224a766ac42..14060532f34 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -1517,7 +1517,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, dns_data = None with plugin_utils.delete_port_on_error( self._core_plugin, context.elevated(), - external_port['id']),\ + external_port['id']), \ db_api.CONTEXT_WRITER.using(context): # Ensure IPv4 addresses are allocated on external port external_ipv4_ips = self._port_ipv4_fixed_ips(external_port) diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index 82144abc6e9..1326a324d1d 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -37,8 +37,6 @@ tests_imports_dot = re.compile(r"\bimport[\s]+neutron.tests\b") tests_imports_from1 = re.compile(r"\bfrom[\s]+neutron.tests\b") tests_imports_from2 = re.compile(r"\bfrom[\s]+neutron[\s]+import[\s]+tests\b") -import_mock = re.compile(r"\bimport[\s]+mock\b") -import_from_mock = re.compile(r"\bfrom[\s]+mock[\s]+import\b") import_six = re.compile(r"\bimport[\s]+six\b") import_from_six = re.compile(r"\bfrom[\s]+six[\s]+import\b") @@ -207,7 +205,7 @@ def check_no_imports_from_tests(logical_line, filename, noqa): for regex in tests_imports_dot, tests_imports_from1, tests_imports_from2: if re.match(regex, logical_line): - yield(0, msg) + yield (0, msg) @core.flake8ext @@ -218,7 +216,7 @@ def check_python3_no_filter(logical_line): "filter(lambda obj: test(obj), data) on python3.") if filter_match.match(logical_line): - yield(0, msg) + yield (0, msg) # TODO(boden): rehome this check to neutron-lib @@ -239,23 +237,6 @@ def check_no_sqlalchemy_event_import(logical_line, filename, noqa): "between unit tests") -@core.flake8ext -def check_no_import_mock(logical_line, filename, noqa): - """N347 - Test code must not import mock library - """ - msg = ("N347: Test code must not import mock library") - - if noqa: - return - - if 'neutron/tests/' not in filename: - return - - for regex in import_mock, import_from_mock: - if re.match(regex, logical_line): - yield(0, msg) - - @core.flake8ext def check_no_import_six(logical_line, filename, noqa): """N348 - Test code must not import six library @@ -267,7 +248,7 @@ def check_no_import_six(logical_line, filename, noqa): for regex in import_six, import_from_six: if re.match(regex, logical_line): - yield(0, msg) + yield (0, msg) @core.flake8ext @@ -281,4 +262,4 @@ def check_no_import_packaging(logical_line, filename, noqa): for regex in import_packaging, import_version_from_packaging: if re.match(regex, logical_line): - yield(0, msg) + yield (0, msg) diff --git a/neutron/notifiers/ironic.py b/neutron/notifiers/ironic.py index 59269c3db64..69b702be909 100644 --- a/neutron/notifiers/ironic.py +++ b/neutron/notifiers/ironic.py @@ -78,7 +78,7 @@ class Notifier(object): os_exc.raise_from_response(response) except Exception as e: LOG.exception('Error encountered posting the event to ' - 'ironic. {error}'.format(error=e)) + 'ironic, error: %s', e) @registry.receives(resources.PORT, [events.AFTER_UPDATE]) def process_port_update_event(self, resource, event, trigger, @@ -102,10 +102,10 @@ class Notifier(object): current_port_status in [n_const.PORT_STATUS_ACTIVE, n_const.PORT_STATUS_ERROR]): port_event = 'bind_port' - LOG.debug('Queuing event for {event_type} for port {port} ' - 'for status {status}.'.format(event_type=port_event, - port=port['id'], - status=current_port_status)) + LOG.debug('Queuing event for %(event_type)s for port %(port)s ' + 'for status %(status)s.', {'event_type': port_event, + 'port': port['id'], + 'status': current_port_status}) if port_event: notify_event = { 'event': '.'.join([BAREMETAL_EVENT_TYPE, port_event]), @@ -132,10 +132,10 @@ class Notifier(object): return port_event = 'delete_port' - LOG.debug('Queuing event for {event_type} for port {port} ' - 'for status {status}.'.format(event_type=port_event, - port=port['id'], - status='DELETED')) + LOG.debug('Queuing event for %(event_type)s for port %(port)s ' + 'for status %(status)s.', {'event_type': port_event, + 'port': port['id'], + 'status': 'DELETED'}) notify_event = { 'event': '.'.join([BAREMETAL_EVENT_TYPE, port_event]), 'port_id': port['id'], diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index 23180665d7e..152314b1ad0 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -647,7 +647,7 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): options = getattr(lsp, 'options') for key in list(options.keys()): if key not in ovn_const.OVN_ROUTER_PORT_OPTION_KEYS: - del(options[key]) + del options[key] return options except idlutils.RowNotFound: return {} diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 6f036a1be4e..9d0ed1b2e13 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -807,7 +807,7 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): for k in (ovn_const.OVN_GW_PORT_EXT_ID_KEY, ovn_const.OVN_GW_NETWORK_EXT_ID_KEY): if k in external_ids: - del(external_ids[k]) + del external_ids[k] cmds.append(self._nb_idl.db_set( 'Logical_Router', lr.uuid, ('external_ids', external_ids))) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index ca9e6be13ac..02817f19206 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -2200,7 +2200,7 @@ class OVNClient(object): options[option] = value else: try: - del(options[option]) + del options[option] except KeyError: # Option not present, job done pass diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index f29d741e4d4..d6d3ff9ec73 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -1866,12 +1866,12 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, if (old_vnic_type != new_vnic_type and binding.vif_type != portbindings.VIF_TYPE_UNBOUND): - LOG.info("Attempting to change VNIC TYPE from {old_type} to " - "{new_type} on port {port_id}, this operation is not " - "allowed because the port is bound".format( - old_type=old_vnic_type, - new_type=new_vnic_type, - port_id=old_port.id)) + LOG.info("Attempting to change VNIC TYPE from %(old_type)s to " + "%(new_type)s on port %(port_id)s, this operation is not " + "allowed because the port is bound", + {'old_type': old_vnic_type, + 'new_type': new_vnic_type, + 'port_id': old_port.id}) raise exc.PortInUse( port_id=old_port.id, net_id=old_port.network_id, diff --git a/neutron/services/placement_report/plugin.py b/neutron/services/placement_report/plugin.py index 33e8c4ba9ee..a00673adff4 100644 --- a/neutron/services/placement_report/plugin.py +++ b/neutron/services/placement_report/plugin.py @@ -171,7 +171,7 @@ class PlacementReportPlugin(service_base.ServicePluginBase): for deferred in deferred_batch: try: - LOG.debug('placement client: {}'.format(deferred)) + LOG.debug('placement client: %s', deferred) deferred.execute() except Exception as e: errors = True diff --git a/neutron/tests/fullstack/test_l3_agent.py b/neutron/tests/fullstack/test_l3_agent.py index 9e42e5b9d97..c392f0ecd02 100644 --- a/neutron/tests/fullstack/test_l3_agent.py +++ b/neutron/tests/fullstack/test_l3_agent.py @@ -289,17 +289,17 @@ class TestL3Agent(base.BaseFullStackTestCase): exception_requests = self._simulate_concurrent_requests_process( funcs, args) - if not all(type(e) == exceptions.BadRequest + if not all(isinstance(e, exceptions.BadRequest) for e in exception_requests): self.fail('Unexpected exception adding interfaces to router from ' 'different subnets overlapping') - if not len(exception_requests) >= (subnets - 1): + if len(exception_requests) < subnets - 1: self.fail('If we have tried to associate %s subnets overlapping ' 'cidr to the router, we should have received at least ' '%s or %s rejected requests, but we have only received ' - '%s', (str(subnets), str(subnets - 1), str(subnets), - str(len(exception_requests)))) + '%s' % (str(subnets), str(subnets - 1), str(subnets), + str(len(exception_requests)))) class TestLegacyL3Agent(TestL3Agent): diff --git a/neutron/tests/functional/cmd/test_status.py b/neutron/tests/functional/cmd/test_status.py index bd063fb93af..8b2d13403ac 100644 --- a/neutron/tests/functional/cmd/test_status.py +++ b/neutron/tests/functional/cmd/test_status.py @@ -68,7 +68,7 @@ class StatusTest(base.BaseLoggingTestCase): self.assertEqual( expected_stderr, stderr.replace('\n', '')) - self.assertTrue(expected_result_title in stdout) + self.assertIn(expected_result_title, stdout) except exceptions.ProcessExecutionError as error: self.fail("neutron-status upgrade check command failed to run. " "Error: %s" % error) diff --git a/neutron/tests/unit/agent/l3/extensions/qos/test_fip.py b/neutron/tests/unit/agent/l3/extensions/qos/test_fip.py index 0d9db5cbc7b..e26c239dcff 100644 --- a/neutron/tests/unit/agent/l3/extensions/qos/test_fip.py +++ b/neutron/tests/unit/agent/l3/extensions/qos/test_fip.py @@ -412,16 +412,16 @@ class RouterFipRateLimitMapsTestCase(base.BaseTestCase): def _check_policy_map_fip(self, router_id, fip_res): if router_id is None: self.assertIsNone(self.policy_map.get_router_id_by_fip(fip_res)) - self.assertTrue(fip_res not in self.policy_map._fips_2_router) + self.assertNotIn(fip_res, self.policy_map._fips_2_router) for router_fips in self.policy_map._router_2_fips.values(): - self.assertTrue(fip_res not in router_fips) + self.assertNotIn(fip_res, router_fips) else: self.assertEqual(router_id, self.policy_map.get_router_id_by_fip(fip_res)) self.assertEqual(router_id, self.policy_map._fips_2_router[fip_res]) - self.assertTrue(fip_res in - self.policy_map._router_2_fips[router_id]) + self.assertIn(fip_res, + self.policy_map._router_2_fips[router_id]) def test_get_router_id_by_fip(self): router_id_1, router_id_2 = _uuid(), _uuid() diff --git a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py b/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py index 26283293370..fbbf3b474fc 100644 --- a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py +++ b/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py @@ -231,7 +231,7 @@ class TestDhcpRpcCallback(base.BaseTestCase): def _make_subnet_dict(subnet): ret = {'id': subnet.id} - if type(subnet.segment_id) == str: + if isinstance(subnet.segment_id, str): ret['segment_id'] = subnet.segment_id return ret diff --git a/neutron/tests/unit/extensions/test_l3_ndp_proxy.py b/neutron/tests/unit/extensions/test_l3_ndp_proxy.py index 1836219cc7b..b6a8fb566a0 100644 --- a/neutron/tests/unit/extensions/test_l3_ndp_proxy.py +++ b/neutron/tests/unit/extensions/test_l3_ndp_proxy.py @@ -277,7 +277,7 @@ class L3NDPProxyTestCase(test_address_scope.AddressScopeTestCase, "The external network %s don't support IPv6 ndp proxy, the " "network has no IPv6 subnets or has no IPv6 address " "scope.") % ext_net['network']['id'] - self.assertTrue(expected_msg in res['NeutronError']['message']) + self.assertIn(expected_msg, res['NeutronError']['message']) router = self._make_router( self.fmt, self._tenant_id, external_gateway_info={'network_id': ext_net['network']['id']}) diff --git a/neutron/tests/unit/extensions/test_securitygroup.py b/neutron/tests/unit/extensions/test_securitygroup.py index 26177f3d8eb..dbb1eb26476 100644 --- a/neutron/tests/unit/extensions/test_securitygroup.py +++ b/neutron/tests/unit/extensions/test_securitygroup.py @@ -838,7 +838,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.fmt, res.get_response(self.ext_api)) secgroup = group['security_group'] - self.assertFalse('security_group_rules' in secgroup) + self.assertNotIn('security_group_rules', secgroup) self.assertEqual(remote_group_id, group['security_group']['id']) # This test case checks that admins from a different tenant can add rules diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index 79fa8818571..336c38aacce 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -206,25 +206,6 @@ class HackingTestCase(base.BaseTestCase): self.assertLinePasses(f, "filter(function, range(0,10))") self.assertLinePasses(f, "lambda x, y: x+y") - def test_check_no_import_mock(self): - pass_line = 'from unittest import mock' - fail_lines = ('import mock', - 'import mock as mock_lib', - 'from mock import patch') - self.assertEqual( - 0, len(list( - checks.check_no_import_mock( - pass_line, "neutron/tests/test_fake.py", None)))) - for fail_line in fail_lines: - self.assertEqual( - 0, len(list( - checks.check_no_import_mock( - fail_line, "neutron/common/utils.py", None)))) - self.assertEqual( - 1, len(list( - checks.check_no_import_mock( - fail_line, "neutron/tests/test_fake.py", None)))) - def test_check_no_import_six(self): pass_line = 'from other_library import six' fail_lines = ('import six', diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index ab2c3b23095..2b5e2d3f4b4 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -354,7 +354,7 @@ class TestMl2NetworksV2(test_plugin.TestNetworksV2, result_macs.append(port_mac) for ip_addr in port.get('fixed_ips'): self.assertIsNone(validators.validate_ip_address(ip_addr)) - self.assertTrue(test_mac in result_macs) + self.assertIn(test_mac, result_macs) def test_bulk_network_before_and_after_events_outside_of_txn(self): # capture session states during each before and after event @@ -1511,7 +1511,7 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase): [mock.call(mock.ANY, [sg]) for sg in used_sg], any_order=True) else: - self.assertTrue('ports' in ports) + self.assertIn('ports', ports) def test_create_ports_bulk_with_portbinding_attrs(self): with self.network() as net: diff --git a/neutron/tests/unit/plugins/ml2/test_port_binding.py b/neutron/tests/unit/plugins/ml2/test_port_binding.py index 6bdc14705b2..5d062324d38 100644 --- a/neutron/tests/unit/plugins/ml2/test_port_binding.py +++ b/neutron/tests/unit/plugins/ml2/test_port_binding.py @@ -481,8 +481,7 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase): 'host-fail') self.assertEqual(webob.exc.HTTPInternalServerError.code, response.status_int) - self.assertTrue(exceptions.PortBindingError.__name__ in - response.text) + self.assertIn(exceptions.PortBindingError.__name__, response.text) def test_create_port_binding_for_non_compute_owner(self): with self.port() as port: @@ -538,8 +537,7 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase): self.host, **kwargs) self.assertEqual(webob.exc.HTTPInternalServerError.code, response.status_int) - self.assertTrue(exceptions.PortBindingError.__name__ in - response.text) + self.assertIn(exceptions.PortBindingError.__name__, response.text) def test_activate_port_binding(self): port, new_binding = self._create_port_and_binding() @@ -613,8 +611,7 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase): response = self._activate_port_binding(port['id'], self.host) self.assertEqual(webob.exc.HTTPInternalServerError.code, response.status_int) - self.assertTrue(exceptions.PortBindingError.__name__ in - response.text) + self.assertIn(exceptions.PortBindingError.__name__, response.text) self.assertEqual(ml2_plugin.MAX_BIND_TRIES, p_mock.call_count) def test_activate_port_binding_non_existing_binding(self): @@ -687,8 +684,7 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase): self.assertEqual(webob.exc.HTTPInternalServerError.code, response.status_int) - self.assertTrue(exceptions.PortBindingError.__name__ in - response.text) + self.assertIn(exceptions.PortBindingError.__name__, response.text) def _create_unbound_port(self): with self.port() as port: diff --git a/neutron/tests/unit/services/placement_report/test_plugin.py b/neutron/tests/unit/services/placement_report/test_plugin.py index d172ea80346..10868877915 100644 --- a/neutron/tests/unit/services/placement_report/test_plugin.py +++ b/neutron/tests/unit/services/placement_report/test_plugin.py @@ -349,6 +349,6 @@ class PlacementReporterAgentsTestCases(test_plugin.Ml2PluginV2TestCase): def test_mechanism_driver_by_agent_type_not_found(self): self.agents = plugin.PlacementReporterAgents(ml2_plugin=self.plugin) self.assertRaises( - Exception, # noqa + KeyError, self.agents.mechanism_driver_by_agent_type, 'agent_not_belonging_to_any_mechanism_driver') diff --git a/tox.ini b/tox.ini index 879fdb46107..0452abb1f65 100644 --- a/tox.ini +++ b/tox.ini @@ -26,7 +26,7 @@ deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt - hacking>=3.0.1,<3.1.0 # Apache-2.0 + hacking>=6.1.0,<6.2.0 # Apache-2.0 allowlist_externals = bash commands = bash {toxinidir}/tools/pip_install_src_modules.sh "{toxinidir}" @@ -116,7 +116,7 @@ deps = {[testenv]deps} bashate>=2.1.1 # Apache-2.0 bandit>=1.7.5 # Apache-2.0 - flake8-import-order==0.18.2 # LGPLv3 + flake8-import-order>=0.18.2,<0.19.0 # LGPLv3 pylint==2.17.4 # GPLv2 commands= # If it is easier to add a check via a shell script, consider adding it in this file @@ -171,13 +171,15 @@ commands = sphinx-build -W -b linkcheck doc/source doc/build/linkcheck [flake8] # E126 continuation line over-indented for hanging indent # E128 continuation line under-indented for visual indent +# E231 missing whitespace after ',' +# E275 missing whitespace after keyword # H405 multi line docstring summary not separated with an empty line # I202 Additional newline in a group of imports # N530 direct neutron imports not allowed # TODO(amotoki) check the following new rules should be fixed or ignored # E731 do not assign a lambda expression, use a def # W504 line break after binary operator -ignore = E126,E128,E731,I202,H405,N530,W504 +ignore = E126,E128,E231,E275,E731,I202,H405,N530,W504 # 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 @@ -202,7 +204,6 @@ extension = N343 = neutron.hacking.checks:check_no_imports_from_tests N344 = neutron.hacking.checks:check_python3_no_filter N346 = neutron.hacking.checks:check_no_sqlalchemy_event_import - N347 = neutron.hacking.checks:check_no_import_mock N348 = neutron.hacking.checks:check_no_import_six N349 = neutron.hacking.checks:check_no_import_packaging # Checks from neutron-lib