From e8f82596b8e46e196c3e577da7078aa1dfe0beb3 Mon Sep 17 00:00:00 2001 From: abhishekkekane Date: Fri, 15 Jan 2016 00:30:56 -0800 Subject: [PATCH] Python3: Replace dict.iteritems with six.iteritems This also adds a check to nova/hacking/checks.py that should check dict.iteritems, dict.itervalues and dict.iterkeys should not be used in the future. NOTE: In _dict_from_object() method of test_db_api.py items() method is called on dict and iteritems() method is called if object is other than dict. Changed it to directly use obj.items as obj can either be dict or object of nova.db.sqlalchemy.models. and both have items() method. Partially-Implements: blueprint nova-python3-mitaka Change-Id: Ib0fe3066199b92e4f013bb15312ada7515fa3d7b --- HACKING.rst | 3 +++ nova/hacking/checks.py | 24 ++++++++++++++++++++++++ nova/tests/unit/db/test_db_api.py | 9 +++------ nova/tests/unit/test_hacking.py | 21 +++++++++++++++++++++ 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 03e2d7ade540..3ec9a2371292 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -54,6 +54,9 @@ Nova Specific Commandments - [N341] contextlib.nested is deprecated - [N342] Config options should be in the central location ``nova/conf/`` - [N343] Check for common double word typos +- [N344] Python 3: do not use dict.iteritems. +- [N345] Python 3: do not use dict.iterkeys. +- [N346] Python 3: do not use dict.itervalues. Creating Unit Tests ------------------- diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index 856eac91daf2..79ed1b676a12 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -591,6 +591,27 @@ def check_doubled_words(physical_line, filename): return (0, msg % {'word': match.group(1)}) +def check_python3_no_iteritems(logical_line): + msg = ("N344: Use six.iteritems() instead of dict.iteritems().") + + if re.search(r".*\.iteritems\(\)", logical_line): + yield(0, msg) + + +def check_python3_no_iterkeys(logical_line): + msg = ("N345: Use six.iterkeys() instead of dict.iterkeys().") + + if re.search(r".*\.iterkeys\(\)", logical_line): + yield(0, msg) + + +def check_python3_no_itervalues(logical_line): + msg = ("N346: Use six.itervalues() instead of dict.itervalues().") + + if re.search(r".*\.itervalues\(\)", logical_line): + yield(0, msg) + + def factory(register): register(import_no_db_in_virt) register(no_db_session_in_public_api) @@ -621,3 +642,6 @@ def factory(register): register(check_greenthread_spawns) register(check_config_option_in_central_place) register(check_doubled_words) + register(check_python3_no_iteritems) + register(check_python3_no_iterkeys) + register(check_python3_no_itervalues) diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 78b6818c63f6..06e0dc530de9 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -1520,11 +1520,8 @@ class ModelsObjectComparatorMixin(object): def _dict_from_object(self, obj, ignored_keys): if ignored_keys is None: ignored_keys = [] - if isinstance(obj, dict): - obj_items = obj.items() - else: - obj_items = obj.iteritems() - return {k: v for k, v in obj_items + + return {k: v for k, v in obj.items() if k not in ignored_keys} def _assertEqualObjects(self, obj1, obj2, ignored_keys=None): @@ -7385,7 +7382,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): service_data['host'] = 'host2' service = db.service_create(self.ctxt, service_data) - existing_node = dict(self.item.iteritems()) + existing_node = dict(self.item.items()) expected = [existing_node] for name in ['bm_node1', 'bm_node2']: diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index a3040e55af23..8093e677a522 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -639,3 +639,24 @@ class HackingTestCase(test.NoDBTestCase): code = "This is the then best comment" self._assert_has_no_errors(code, checks.check_doubled_words) + + def test_dict_iteritems(self): + self.assertEqual(1, len(list(checks.check_python3_no_iteritems( + "obj.iteritems()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iteritems( + "six.iteritems(ob))")))) + + def test_dict_iterkeys(self): + self.assertEqual(1, len(list(checks.check_python3_no_iterkeys( + "obj.iterkeys()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iterkeys( + "six.iterkeys(ob))")))) + + def test_dict_itervalues(self): + self.assertEqual(1, len(list(checks.check_python3_no_itervalues( + "obj.itervalues()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_itervalues( + "six.itervalues(ob))"))))