Skip to content

Commit 9100ea9

Browse files
committed
Fix unordered dict bug in to_dict()
Issues: Fixes #687 Problem: On python3 (and also I presume on python2) the test_two_refs test in test_mixins.py will fail because the returned value is {"x": [TraversalRecord, "z"], "z": [1, "a"]} instead of the expected value {"x": [1, "a"], "z": ["TraversalRecord", "x"]} Analysis: I tracked this down to the `_traverse_dict()` method where it iterates over the values of the `instance_dict` variable. The problem is that because dictionaries are unordered, the first value that is iterated upon will sometimes return "z" first instead of "x". This causes the result to become "turned around" and the test to fail. I never saw this crop up on python 2, but it crops up regularly on python 3. The fix I applied is to cast the unordered dict in `_traverse_dict` into an OrderedDict. Doing this makes the values of that variable always known, so-to-speak, because they have a defined order. Therefore the tests will pass as expected. Tests: * f5/bigip/test/test_mixins.py::test_to_refs
1 parent 3882277 commit 9100ea9

2 files changed

Lines changed: 14 additions & 1 deletion

File tree

f5/bigip/mixins.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,14 @@
1717
# NOTE: Code taken from Effective Python Item 26
1818

1919

20+
try:
21+
from collections import OrderedDict
22+
except ImportError:
23+
from ordereddict import OrderedDict
24+
2025
from distutils.version import LooseVersion
26+
from six import iteritems
27+
2128
import logging
2229

2330
from f5.sdk_exception import F5SDKError
@@ -57,7 +64,11 @@ def _to_dict(self):
5764

5865
def _traverse_dict(self, instance_dict):
5966
output = {}
60-
for key, value in instance_dict.items():
67+
68+
# This iteration breaks if the second value comes before
69+
# the first. We must use ordered dicts here
70+
tmp = OrderedDict(sorted(iteritems(instance_dict), key=lambda t: t[0]))
71+
for key, value in iteritems(tmp):
6172
output[key] = self._traverse(key, value)
6273
return output
6374

@@ -68,6 +79,7 @@ def _traverse(self, key, value):
6879
return ToDictMixin.traversed[id(value)]
6980
else:
7081
ToDictMixin.traversed[id(value)] = ['TraversalRecord', key]
82+
7183
if isinstance(value, ToDictMixin):
7284
return value._to_dict()
7385
elif isinstance(value, dict):

requirements.test.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ pytest-cov>=2.2.1
1010
git+https://github.com/F5Networks/pytest-symbols.git
1111
python-coveralls
1212
pyopenssl
13+
ordereddict

0 commit comments

Comments
 (0)