From 5836bbca83845fd78200c083465601d2558cdac2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Adrien=20Verg=C3=A9?= Date: Tue, 19 May 2015 11:05:27 +0200 Subject: [PATCH] Python 3: Use six.moves.range The function `xrange` was renamed to `range` in Python 3. * Remove `xrange` occurences so that Python 3 tests can pass. Use `six.moves.range` instead to get the right function in both cases. * Generalize the use of the efficient `range` (ex-`xrange`) in critical sections (when iterating over large lists). * Simplify code. * Add a hacking check to prevent future usage of `xrange`. Change-Id: I080acaaa1d4753619fbbb76dddba6d946d84e73f Partially implements: blueprint neutron-python3 --- HACKING.rst | 1 + neutron/db/sqlalchemyutils.py | 2 +- neutron/hacking/checks.py | 7 ++++ neutron/plugins/brocade/vlanbm.py | 2 +- .../agent/linuxbridge_neutron_agent.py | 2 +- neutron/plugins/ml2/drivers/type_gre.py | 2 +- neutron/plugins/ml2/drivers/type_vlan.py | 2 +- neutron/plugins/ml2/drivers/type_vxlan.py | 2 +- .../openvswitch/agent/ovs_neutron_agent.py | 4 +-- .../agent/linux/test_async_process.py | 4 +-- .../agent/linux/test_process_monitor.py | 2 +- neutron/tests/tempest/common/glance_http.py | 4 +-- neutron/tests/unit/agent/l3/test_agent.py | 6 ++-- neutron/tests/unit/api/v2/test_base.py | 3 +- neutron/tests/unit/hacking/test_checks.py | 34 ++++++++++++------- .../unit/plugins/cisco/n1kv/test_n1kv_db.py | 4 +-- .../plugins/ml2/drivers/base_type_tunnel.py | 4 +-- .../plugins/oneconvergence/test_nvsd_agent.py | 5 +-- .../unit/tests/test_post_mortem_debug.py | 2 +- 19 files changed, 55 insertions(+), 37 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 35281536c..ce05f7f15 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -14,6 +14,7 @@ Neutron Specific Commandments - [N322] Detect common errors with assert_called_once_with - [N323] Enforce namespace-less imports for oslo libraries - [N324] Prevent use of deprecated contextlib.nested. +- [N325] Python 3: Do not use xrange. Creating Unit Tests ------------------- diff --git a/neutron/db/sqlalchemyutils.py b/neutron/db/sqlalchemyutils.py index f3df78fce..466cb3fd9 100644 --- a/neutron/db/sqlalchemyutils.py +++ b/neutron/db/sqlalchemyutils.py @@ -88,7 +88,7 @@ def paginate_query(query, model, limit, sorts, marker_obj=None): criteria_list = [] for i, sort in enumerate(sorts): crit_attrs = [(getattr(model, sorts[j][0]) == marker_values[j]) - for j in moves.xrange(i)] + for j in moves.range(i)] model_attr = getattr(model, sort[0]) if sort[1]: crit_attrs.append((model_attr > marker_values[i])) diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index 062f83961..10b26050f 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -160,6 +160,12 @@ def check_no_contextlib_nested(logical_line, filename): yield(0, msg) +def check_python3_xrange(logical_line): + if re.search(r"\bxrange\s*\(", logical_line): + yield(0, "N325: Do not use xrange. Use range, or six.moves.range for " + "large loops.") + + def factory(register): register(validate_log_translations) register(use_jsonutils) @@ -167,3 +173,4 @@ def factory(register): register(no_translate_debug_logs) register(check_oslo_namespace_imports) register(check_no_contextlib_nested) + register(check_python3_xrange) diff --git a/neutron/plugins/brocade/vlanbm.py b/neutron/plugins/brocade/vlanbm.py index fb3aeba20..3323044c4 100644 --- a/neutron/plugins/brocade/vlanbm.py +++ b/neutron/plugins/brocade/vlanbm.py @@ -42,7 +42,7 @@ class VlanBitmap(object): min_vlan_search = vlan_id or MIN_VLAN max_vlan_search = (vlan_id + 1) if vlan_id else MAX_VLAN - for vlan in moves.xrange(min_vlan_search, max_vlan_search): + for vlan in moves.range(min_vlan_search, max_vlan_search): if vlan not in self.vlans: self.vlans.add(vlan) return vlan diff --git a/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py b/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py index fa7c4b906..604cf73c4 100644 --- a/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py +++ b/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py @@ -531,7 +531,7 @@ class LinuxBridgeManager(object): return False test_iface = None - for seg_id in moves.xrange(1, p_const.MAX_VXLAN_VNI + 1): + for seg_id in moves.range(1, p_const.MAX_VXLAN_VNI + 1): if not ip_lib.device_exists( self.get_vxlan_device_name(seg_id)): test_iface = self.ensure_vxlan(seg_id) diff --git a/neutron/plugins/ml2/drivers/type_gre.py b/neutron/plugins/ml2/drivers/type_gre.py index fe9b87a76..134348b06 100644 --- a/neutron/plugins/ml2/drivers/type_gre.py +++ b/neutron/plugins/ml2/drivers/type_gre.py @@ -93,7 +93,7 @@ class GreTypeDriver(type_tunnel.TunnelTypeDriver): "%(tun_min)s:%(tun_max)s"), {'tun_min': tun_min, 'tun_max': tun_max}) else: - gre_ids |= set(moves.xrange(tun_min, tun_max + 1)) + gre_ids |= set(moves.range(tun_min, tun_max + 1)) session = db_api.get_session() try: diff --git a/neutron/plugins/ml2/drivers/type_vlan.py b/neutron/plugins/ml2/drivers/type_vlan.py index d2459e785..451e80187 100644 --- a/neutron/plugins/ml2/drivers/type_vlan.py +++ b/neutron/plugins/ml2/drivers/type_vlan.py @@ -115,7 +115,7 @@ class VlanTypeDriver(helpers.SegmentTypeDriver): # this physical network vlan_ids = set() for vlan_min, vlan_max in vlan_ranges: - vlan_ids |= set(moves.xrange(vlan_min, vlan_max + 1)) + vlan_ids |= set(moves.range(vlan_min, vlan_max + 1)) # remove from table unallocated vlans not currently # allocatable diff --git a/neutron/plugins/ml2/drivers/type_vxlan.py b/neutron/plugins/ml2/drivers/type_vxlan.py index 5b7dbdc38..51125701c 100644 --- a/neutron/plugins/ml2/drivers/type_vxlan.py +++ b/neutron/plugins/ml2/drivers/type_vxlan.py @@ -96,7 +96,7 @@ class VxlanTypeDriver(type_tunnel.TunnelTypeDriver): "%(tun_min)s:%(tun_max)s"), {'tun_min': tun_min, 'tun_max': tun_max}) else: - vxlan_vnis |= set(moves.xrange(tun_min, tun_max + 1)) + vxlan_vnis |= set(moves.range(tun_min, tun_max + 1)) session = db_api.get_session() with session.begin(subtransactions=True): diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 182a8fd8c..bc42cd85f 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -161,8 +161,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, super(OVSNeutronAgent, self).__init__() self.use_veth_interconnection = use_veth_interconnection self.veth_mtu = veth_mtu - self.available_local_vlans = set(moves.xrange(p_const.MIN_VLAN_TAG, - p_const.MAX_VLAN_TAG)) + self.available_local_vlans = set(moves.range(p_const.MIN_VLAN_TAG, + p_const.MAX_VLAN_TAG)) self.use_call = True self.tunnel_types = tunnel_types or [] self.l2_pop = l2_population diff --git a/neutron/tests/functional/agent/linux/test_async_process.py b/neutron/tests/functional/agent/linux/test_async_process.py index 8fddb6d4b..89c2bec89 100644 --- a/neutron/tests/functional/agent/linux/test_async_process.py +++ b/neutron/tests/functional/agent/linux/test_async_process.py @@ -14,8 +14,6 @@ import eventlet -from six import moves - from neutron.agent.linux import async_process from neutron.tests import base @@ -25,7 +23,7 @@ class AsyncProcessTestFramework(base.BaseTestCase): def setUp(self): super(AsyncProcessTestFramework, self).setUp() self.test_file_path = self.get_temp_file_path('test_async_process.tmp') - self.data = [str(x) for x in moves.xrange(4)] + self.data = [str(x) for x in range(4)] with file(self.test_file_path, 'w') as f: f.writelines('%s\n' % item for item in self.data) diff --git a/neutron/tests/functional/agent/linux/test_process_monitor.py b/neutron/tests/functional/agent/linux/test_process_monitor.py index 51bf79668..0802c3a45 100644 --- a/neutron/tests/functional/agent/linux/test_process_monitor.py +++ b/neutron/tests/functional/agent/linux/test_process_monitor.py @@ -56,7 +56,7 @@ class BaseTestProcessMonitor(base.BaseTestCase): def spawn_n_children(self, n, service=None): self._child_processes = [] - for child_number in moves.xrange(n): + for child_number in moves.range(n): uuid = self._child_uuid(child_number) _callback = self._make_cmdline_callback(uuid) pm = external_process.ProcessManager( diff --git a/neutron/tests/tempest/common/glance_http.py b/neutron/tests/tempest/common/glance_http.py index 665047739..6cdbadc3b 100644 --- a/neutron/tests/tempest/common/glance_http.py +++ b/neutron/tests/tempest/common/glance_http.py @@ -17,7 +17,6 @@ import copy import hashlib -from six.moves import http_client as httplib import json import posixpath import re @@ -30,6 +29,7 @@ import urlparse import OpenSSL from oslo_log import log as logging from six import moves +from six.moves import http_client as httplib from tempest_lib import exceptions as lib_exc from neutron.tests.tempest import exceptions as exc @@ -260,7 +260,7 @@ class VerifiedHTTPSConnection(httplib.HTTPSConnection): # Also try Subject Alternative Names for a match san_list = None - for i in moves.xrange(x509.get_extension_count()): + for i in moves.range(x509.get_extension_count()): ext = x509.get_extension(i) if ext.get_short_name() == 'subjectAltName': san_list = str(ext) diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 5f0852b2e..e628cd465 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -22,6 +22,7 @@ import mock import netaddr from oslo_log import log import oslo_messaging +from six import moves from testtools import matchers from neutron.agent.common import config as agent_config @@ -115,7 +116,8 @@ def router_append_subnet(router, count=1, ip_version=4, ipv6_subnet_modes = [subnet_mode_none] * count elif len(ipv6_subnet_modes) != count: ipv6_subnet_modes.extend([subnet_mode_none for i in - xrange(len(ipv6_subnet_modes), count)]) + moves.range(len(ipv6_subnet_modes), + count)]) if ip_version == 4: ip_pool = '35.4.%i.4' @@ -144,7 +146,7 @@ def router_append_subnet(router, count=1, ip_version=4, fixed_ips, subnets = [], [] num_existing_subnets = len(subnets) - for i in xrange(count): + for i in moves.range(count): subnet_id = _uuid() fixed_ips.append( {'ip_address': ip_pool % (i + num_existing_subnets), diff --git a/neutron/tests/unit/api/v2/test_base.py b/neutron/tests/unit/api/v2/test_base.py index fb7828b7b..60201e40f 100644 --- a/neutron/tests/unit/api/v2/test_base.py +++ b/neutron/tests/unit/api/v2/test_base.py @@ -17,6 +17,7 @@ import os import mock from oslo_config import cfg +from six import moves import six.moves.urllib.parse as urlparse import webob from webob import exc @@ -1302,7 +1303,7 @@ class DHCPNotificationTest(APIv2TestBase): resource += 's' num = len(initial_input[resource]) if initial_input and isinstance( initial_input[resource], list) else 1 - expected = [expected_item for x in xrange(num)] + expected = [expected_item for x in moves.range(num)] self.assertEqual(expected, dhcp_notifier.call_args_list) self.assertEqual(num, dhcp_notifier.call_count) self.assertEqual(expected_code, res.status_int) diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index b87ad18bc..1445923b9 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -18,6 +18,13 @@ from neutron.tests import base class HackingTestCase(base.BaseTestCase): + def assertLinePasses(self, func, line): + with testtools.ExpectedException(StopIteration): + next(func(line)) + + def assertLineFails(self, func, line): + self.assertIsInstance(next(func(line)), tuple) + def test_log_translations(self): expected_marks = { 'error': '_LE', @@ -108,16 +115,17 @@ class HackingTestCase(base.BaseTestCase): "neutron/tests/test_assert.py")))) def test_check_oslo_namespace_imports(self): - def check(s, fail=True): - func = checks.check_oslo_namespace_imports - if fail: - self.assertIsInstance(next(func(s)), tuple) - else: - with testtools.ExpectedException(StopIteration): - next(func(s)) - - check('from oslo_utils import importutils', fail=False) - check('import oslo_messaging', fail=False) - check('from oslo.utils import importutils') - check('from oslo import messaging') - check('import oslo.messaging') + f = checks.check_oslo_namespace_imports + self.assertLinePasses(f, 'from oslo_utils import importutils') + self.assertLinePasses(f, 'import oslo_messaging') + self.assertLineFails(f, 'from oslo.utils import importutils') + self.assertLineFails(f, 'from oslo import messaging') + self.assertLineFails(f, 'import oslo.messaging') + + def test_check_python3_xrange(self): + f = checks.check_python3_xrange + self.assertLineFails(f, 'a = xrange(1000)') + self.assertLineFails(f, 'b =xrange ( 42 )') + self.assertLineFails(f, 'c = xrange(1, 10, 2)') + self.assertLinePasses(f, 'd = range(1000)') + self.assertLinePasses(f, 'e = six.moves.range(1337)') diff --git a/neutron/tests/unit/plugins/cisco/n1kv/test_n1kv_db.py b/neutron/tests/unit/plugins/cisco/n1kv/test_n1kv_db.py index eadc4b4a3..fbe322330 100644 --- a/neutron/tests/unit/plugins/cisco/n1kv/test_n1kv_db.py +++ b/neutron/tests/unit/plugins/cisco/n1kv/test_n1kv_db.py @@ -140,7 +140,7 @@ class VlanAllocationsTest(testlib_api.SqlTestCase): def test_vlan_pool(self): vlan_ids = set() - for x in moves.xrange(VLAN_MIN, VLAN_MAX + 1): + for x in moves.range(VLAN_MIN, VLAN_MAX + 1): (physical_network, seg_type, vlan_id, m_ip) = n1kv_db_v2.reserve_vlan(self.session, self.net_p) self.assertEqual(physical_network, PHYS_NET) @@ -233,7 +233,7 @@ class VxlanAllocationsTest(testlib_api.SqlTestCase, def test_vxlan_pool(self): vxlan_ids = set() - for x in moves.xrange(VXLAN_MIN, VXLAN_MAX + 1): + for x in moves.range(VXLAN_MIN, VXLAN_MAX + 1): vxlan = n1kv_db_v2.reserve_vxlan(self.session, self.net_p) vxlan_id = vxlan[2] self.assertThat(vxlan_id, matchers.GreaterThan(VXLAN_MIN - 1)) diff --git a/neutron/tests/unit/plugins/ml2/drivers/base_type_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/base_type_tunnel.py index 6637c6035..cd5469fd0 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/base_type_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/base_type_tunnel.py @@ -139,7 +139,7 @@ class TunnelTypeTestMixin(object): api.PHYSICAL_NETWORK: 'None', api.SEGMENTATION_ID: None} - for x in moves.xrange(TUN_MIN, TUN_MAX + 1): + for x in moves.range(TUN_MIN, TUN_MAX + 1): segment = self.driver.reserve_provider_segment(self.session, specs) self.assertEqual(self.TYPE, segment[api.NETWORK_TYPE]) @@ -170,7 +170,7 @@ class TunnelTypeTestMixin(object): def test_allocate_tenant_segment(self): tunnel_ids = set() - for x in moves.xrange(TUN_MIN, TUN_MAX + 1): + for x in moves.range(TUN_MIN, TUN_MAX + 1): segment = self.driver.allocate_tenant_segment(self.session) self.assertThat(segment[api.SEGMENTATION_ID], matchers.GreaterThan(TUN_MIN - 1)) diff --git a/neutron/tests/unit/plugins/oneconvergence/test_nvsd_agent.py b/neutron/tests/unit/plugins/oneconvergence/test_nvsd_agent.py index 6cd2615e1..5835d89a7 100644 --- a/neutron/tests/unit/plugins/oneconvergence/test_nvsd_agent.py +++ b/neutron/tests/unit/plugins/oneconvergence/test_nvsd_agent.py @@ -17,6 +17,7 @@ import time import mock from oslo_config import cfg +from six import moves import testtools from neutron.agent.common import ovs_lib @@ -125,8 +126,8 @@ class TestNVSDAgent(TestOneConvergenceAgentBase): # Ensure vif_ports_scenario is longer than DAEMON_LOOP_COUNT if len(self.vif_ports_scenario) < DAEMON_LOOP_COUNT: self.vif_ports_scenario.extend( - [] for _i in xrange(DAEMON_LOOP_COUNT - - len(self.vif_ports_scenario))) + [] for _i in moves.range(DAEMON_LOOP_COUNT - + len(self.vif_ports_scenario))) with contextlib.nested( mock.patch.object(time, 'sleep', side_effect=sleep_mock), diff --git a/neutron/tests/unit/tests/test_post_mortem_debug.py b/neutron/tests/unit/tests/test_post_mortem_debug.py index 1d940f4fa..652bdee6b 100644 --- a/neutron/tests/unit/tests/test_post_mortem_debug.py +++ b/neutron/tests/unit/tests/test_post_mortem_debug.py @@ -77,7 +77,7 @@ class TestGetIgnoredTraceback(base.BaseTestCase): tb = root_tb tracebacks = [tb] - for x in moves.xrange(len(ignored_bit_array) - 1): + for x in moves.range(len(ignored_bit_array) - 1): tb.tb_next = mock.Mock() tb = tb.tb_next tracebacks.append(tb) -- 2.45.2