Merge pull request #692 from caphrim007/bugfix-more-python-3-fixes

Adds some more python 2 to 3 fixes

Author: zancas

f5/bigip/resource.py

@@ -101,6 +101,9 @@
 from f5.sdk_exception import F5SDKError
 from f5.sdk_exception import UnsupportedMethod
 from requests.exceptions import HTTPError
+from six import iteritems
+from six import iterkeys
+from six import itervalues
 
 
 class MissingRequiredCommandParameter(F5SDKError):
@@ -195,7 +198,7 @@ def _missing_required_parameters(rqset, **kwargs):
 
     ::returns list
     """
-    key_set = set(kwargs.keys())
+    key_set = set(list(iterkeys(kwargs)))
     required_minus_received = rqset - key_set
     if required_minus_received != set():
         return list(required_minus_received)
@@ -307,7 +310,7 @@ def _check_exclusive_parameters(self, **kwargs):
         :raises ExclusiveAttributesPresent
         """
         if len(self._meta_data['exclusive_attributes']) > 0:
-            attr_set = set(kwargs.keys())
+            attr_set = set(list(iterkeys(kwargs)))
             ex_set = set(self._meta_data['exclusive_attributes'][0])
             common_set = attr_set.intersection(ex_set)
             if len(common_set) > 1:
@@ -479,9 +482,17 @@ def _update(self, **kwargs):
         # because these are subCollections and _meta_data and
         # other non-BIG-IP® attrs are not removed from the subCollections
         # See issue #146 for details
-        for key, value in self.__dict__.items():
+        tmp = dict()
+        for key, value in iteritems(self.__dict__):
+            # In Python2 versions we were changing a dictionary in place,
+            # but this cannot be done with an iterator as an error is raised.
+            # So instead we create a temporary holder for the modified dict
+            # and then re-assign it afterwards.
             if isinstance(value, Collection):
-                self.__dict__.pop(key, '')
+                pass
+            else:
+                tmp[key] = value
+        self.__dict__ = tmp
         data_dict = self.to_dict()
 
         # Remove any read-only attributes from our data_dict before we update
@@ -773,7 +784,7 @@ def _activate_URI(self, selfLinkuri):
 
         # attrs local alias
         attribute_reg = self._meta_data.get('attribute_registry', {})
-        attrs = attribute_reg.values()
+        attrs = list(itervalues(attribute_reg))
         attrs.append(Stats)
 
         (scheme, domain, path, qarg, frag) = urlparse.urlsplit(selfLinkuri)

f5/bigip/test/test_resource.py

@@ -81,22 +81,22 @@ def test_Resource__local_update_IncompatibleKeys():
     r = Resource(mock.MagicMock())
     with pytest.raises(DeviceProvidesIncompatibleKey) as DPIKIO:
         r._local_update({"_meta_data": "foo"})
-    assert DPIKIO.value.message ==\
+    assert str(DPIKIO.value) ==\
         "Response contains key '_meta_data' which is incompatible"\
         " with this API!!\n Response json: {'_meta_data': 'foo'}"
     with pytest.raises(DeviceProvidesIncompatibleKey) as DPIKIO:
         r._local_update({"__MANGLENAME": "foo"})
-    assert DPIKIO.value.message ==\
+    assert str(DPIKIO.value) ==\
         "Device provided '__MANGLENAME' which is disallowed,"\
         " it mangles into a Python non-public attribute."
     with pytest.raises(DeviceProvidesIncompatibleKey) as DPIKIO:
         r._local_update({"for": "foo"})
-    assert DPIKIO.value.message ==\
+    assert str(DPIKIO.value) ==\
         "Device provided 'for' which is disallowed because"\
         " it's a Python keyword."
     with pytest.raises(DeviceProvidesIncompatibleKey) as DPIKIO:
         r._local_update({"%abcd": "foo"})
-    assert DPIKIO.value.message ==\
+    assert str(DPIKIO.value) ==\
         "Device provided '%abcd' which is disallowed because"\
         " it's not a valid Python 2.7 identifier."
 
@@ -132,19 +132,26 @@ def test_missing_required_creation_parameter(self):
         r._meta_data['required_creation_parameters'] = set(['NONEMPTY'])
         with pytest.raises(MissingRequiredCreationParameter) as MRCPEIO:
             r.create(partition="Common", name='CreateTest')
-        assert MRCPEIO.value.message ==\
+        assert str(MRCPEIO.value) ==\
             "Missing required params: ['NONEMPTY']"
 
     def test_KindTypeMismatch(self):
+        expected_result = (
+            "For instances of type ''Virtual'' the "
+            "corresponding kind must be ''tm:ltm:virtual:virtualstate'' "
+            "but creation returned "
+            "JSON with kind: 'tm:'"
+        )
         r = Virtual(mock.MagicMock())
+
         r._meta_data['bigip']._meta_data['icr_session'].post.return_value =\
-            MockResponse({u"kind": u"tm:"})
-        with pytest.raises(KindTypeMismatch) as KTMmEIO:
+            MockResponse({"kind": "tm:"})
+
+        with pytest.raises(KindTypeMismatch) as error:
             r.create(partition="Common", name="test_create")
-        assert KTMmEIO.value.message ==\
-            "For instances of type ''Virtual'' the corresponding kind must "\
-            "be ''tm:ltm:virtual:virtualstate'' but creation returned "\
-            "JSON with kind: u'tm:'"
+
+        result = error.value.args[0]
+        assert result == expected_result
 
     def test_success(self, fake_vs):
         x = fake_vs.create(partition="Common", name="test_create")
@@ -217,7 +224,7 @@ def test__create_with_Collision():
     r._meta_data['uri'] = 'URI'
     with pytest.raises(URICreationCollision) as UCCEIO:
         r.create(uri='URI')
-    assert UCCEIO.value.message ==\
+    assert str(UCCEIO.value) ==\
         "There was an attempt to assign a new uri to this resource,"\
         " the _meta_data['uri'] is URI and it should not be changed."
 
@@ -235,7 +242,7 @@ def test__check_generation_with_mismatch(self):
         r.generation = 1
         with pytest.raises(GenerationMismatch) as GMEIO:
             r.update(a=u"b", force=False)
-        assert GMEIO.value.message ==\
+        assert str(GMEIO.value) ==\
             'The generation of the object on the BigIP (0)'\
             ' does not match the current object(1)'
 
@@ -267,7 +274,6 @@ def test_Collection_removal(self):
         r.update(a=u"b")
         submitted = r._meta_data['bigip']. \
             _meta_data['icr_session'].put.call_args[1]['json']
-
         assert 'contained' not in submitted
 
     def test_read_only_removal(self):
@@ -366,7 +372,7 @@ def test_read_only_validate(self):
         r.generation = 0
         with pytest.raises(AttemptedMutationOfReadOnly) as AMOROEIO:
             r.modify(READONLY=True)
-        assert "READONLY" in AMOROEIO.value.message
+        assert "READONLY" in str(AMOROEIO.value)
 
     def test_reduce_boolean_removes_enabled(self, fake_rsrc):
         fake_rsrc.modify(enabled=False)
@@ -425,7 +431,7 @@ def test_invalid_force(self):
         r._meta_data['bigip']._meta_data = {'icr_session': mock_session}
         with pytest.raises(InvalidForceType) as IFTEIO:
             r.delete(force='true')
-        assert IFTEIO.value.message == 'force parameter must be type bool'
+        assert str(IFTEIO.value) == 'force parameter must be type bool'
 
 
 class Element(Resource):
@@ -469,7 +475,7 @@ def test_unregistered_kind(self):
         c.generation = 0
         with pytest.raises(UnregisteredKind) as UKEIO:
             c.get_collection()
-        assert UKEIO.value.message ==\
+        assert str(UKEIO.value) ==\
             "'tm:' is not registered!"
 
 
@@ -479,15 +485,15 @@ def test_missing_required_params(self):
         r._meta_data['required_load_parameters'] = set(['IMPOSSIBLE'])
         with pytest.raises(MissingRequiredReadParameter) as MRREIO:
             r.load(partition='Common', name='test_load')
-        assert MRREIO.value.message ==\
+        assert str(MRREIO.value) ==\
             "Missing required params: ['IMPOSSIBLE']"
 
     def test_requests_params_collision(self):
         r = Resource(mock.MagicMock())
         with pytest.raises(RequestParamKwargCollision) as RPKCEIO:
             r.load(partition='Common', name='test_load',
                    requests_params={'partition': 'ERROR'})
-        assert RPKCEIO.value.message ==\
+        assert str(RPKCEIO.value) ==\
             "Requests Parameter 'partition' collides with a load parameter"\
             " of the same name."