Repository: softlayer/softlayer-python
Pull request #916 (2017-12-27 22:38:26)

requests sessions and retry
API calls go through Requests.session, and will automatically retry on connection errors.


Patch:
diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index c0971279..4de814b5 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -29,3 +29,4 @@ Swapnil Khanapurkar <swapnil_khanapurkar@persistent.co.in>
 The SoftLayer Developer Network <sldn@softlayer.com>
 Tim Ariyeh <tim.ariyeh@gmail.com>
 Wissam Elriachy <wissam.elriachy@gmail.com>
+Anthony Monthe (ZuluPro) <anthony.monthe@gmail.com>
diff --git a/SoftLayer/fixtures/SoftLayer_Account.py b/SoftLayer/fixtures/SoftLayer_Account.py
index 0f3a0a6a..4a59000b 100644
--- a/SoftLayer/fixtures/SoftLayer_Account.py
+++ b/SoftLayer/fixtures/SoftLayer_Account.py
@@ -266,7 +266,8 @@
             "statusId": 4,
             "accountId": 1234
         }
-    ]
+    ],
+    'accountId': 1234
 }
 
 getRwhoisData = {
diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py
index 32bda03e..97d67a5b 100644
--- a/SoftLayer/transports.py
+++ b/SoftLayer/transports.py
@@ -11,6 +11,8 @@
 import time
 
 import requests
+from requests.adapters import HTTPAdapter
+from urllib3.util.retry import Retry
 
 from SoftLayer import consts
 from SoftLayer import exceptions
@@ -38,6 +40,20 @@
 }
 
 
+def get_session(user_agent):
+    """Sets up urllib sessions"""
+
+    client = requests.Session()
+    client.headers.update({
+        'Content-Type': 'application/json',
+        'User-Agent': user_agent,
+    })
+    retry = Retry(connect=3, backoff_factor=3)
+    adapter = HTTPAdapter(max_retries=retry)
+    client.mount('https://', adapter)
+    return client
+
+
 class Request(object):
     """Transport request object."""
 
@@ -107,6 +123,15 @@ def __init__(self, endpoint_url=None, timeout=None, proxy=None, user_agent=None,
         self.proxy = proxy
         self.user_agent = user_agent or consts.USER_AGENT
         self.verify = verify
+        self._client = None
+
+    @property
+    def client(self):
+        """Returns client session object"""
+
+        if self._client is None:
+            self._client = get_session(self.user_agent)
+        return self._client
 
     def __call__(self, request):
         """Makes a SoftLayer API call against the XML-RPC endpoint.
@@ -154,13 +179,13 @@ def __call__(self, request):
         LOGGER.debug(payload)
 
         try:
-            resp = requests.request('POST', url,
-                                    data=payload,
-                                    headers=request.transport_headers,
-                                    timeout=self.timeout,
-                                    verify=verify,
-                                    cert=request.cert,
-                                    proxies=_proxies_dict(self.proxy))
+            resp = self.client.request('POST', url,
+                                       data=payload,
+                                       headers=request.transport_headers,
+                                       timeout=self.timeout,
+                                       verify=verify,
+                                       cert=request.cert,
+                                       proxies=_proxies_dict(self.proxy))
             LOGGER.debug("=== RESPONSE ===")
             LOGGER.debug(resp.headers)
             LOGGER.debug(resp.content)
@@ -208,6 +233,15 @@ def __init__(self, endpoint_url=None, timeout=None, proxy=None, user_agent=None,
         self.proxy = proxy
         self.user_agent = user_agent or consts.USER_AGENT
         self.verify = verify
+        self._client = None
+
+    @property
+    def client(self):
+        """Returns client session object"""
+
+        if self._client is None:
+            self._client = get_session(self.user_agent)
+        return self._client
 
     def __call__(self, request):
         """Makes a SoftLayer API call against the REST endpoint.
@@ -217,9 +251,6 @@ def __call__(self, request):
 
         :param request request: Request object
         """
-        request.transport_headers.setdefault('Content-Type', 'application/json')
-        request.transport_headers.setdefault('User-Agent', self.user_agent)
-
         params = request.headers.copy()
         if request.mask:
             params['objectMask'] = _format_object_mask(request.mask)
@@ -275,15 +306,15 @@ def __call__(self, request):
         LOGGER.debug(request.transport_headers)
         LOGGER.debug(raw_body)
         try:
-            resp = requests.request(method, url,
-                                    auth=auth,
-                                    headers=request.transport_headers,
-                                    params=params,
-                                    data=raw_body,
-                                    timeout=self.timeout,
-                                    verify=verify,
-                                    cert=request.cert,
-                                    proxies=_proxies_dict(self.proxy))
+            resp = self.client.request(method, url,
+                                       auth=auth,
+                                       headers=request.transport_headers,
+                                       params=params,
+                                       data=raw_body,
+                                       timeout=self.timeout,
+                                       verify=verify,
+                                       cert=request.cert,
+                                       proxies=_proxies_dict(self.proxy))
             LOGGER.debug("=== RESPONSE ===")
             LOGGER.debug(resp.headers)
             LOGGER.debug(resp.text)
@@ -339,13 +370,11 @@ def __call__(self, call):
             module_path = 'SoftLayer.fixtures.%s' % call.service
             module = importlib.import_module(module_path)
         except ImportError:
-            raise NotImplementedError('%s fixture is not implemented'
-                                      % call.service)
+            raise NotImplementedError('%s fixture is not implemented' % call.service)
         try:
             return getattr(module, call.method)
         except AttributeError:
-            raise NotImplementedError('%s::%s fixture is not implemented'
-                                      % (call.service, call.method))
+            raise NotImplementedError('%s::%s fixture is not implemented' % (call.service, call.method))
 
 
 def _proxies_dict(proxy):
diff --git a/setup.py b/setup.py
index fb252243..23db6ab6 100644
--- a/setup.py
+++ b/setup.py
@@ -33,9 +33,10 @@
         'six >= 1.7.0',
         'prettytable >= 0.7.0',
         'click >= 5',
-        'requests >= 2.7.0',
+        'requests >= 2.18.4',
         'prompt_toolkit >= 0.53',
         'pygments >= 2.0.0',
+        'urllib3 >= 1.22'
     ],
     keywords=['softlayer', 'cloud'],
     classifiers=[
diff --git a/tools/requirements.txt b/tools/requirements.txt
index 6eb1237b..4663c430 100644
--- a/tools/requirements.txt
+++ b/tools/requirements.txt
@@ -3,3 +3,4 @@ click >= 5
 prettytable >= 0.7.0
 six >= 1.7.0
 prompt_toolkit
+urllib3


Test patch:
diff --git a/tests/decoration_tests.py b/tests/decoration_tests.py
index 785d4758..9d230671 100644
--- a/tests/decoration_tests.py
+++ b/tests/decoration_tests.py
@@ -7,7 +7,6 @@
 
 import logging
 import mock
-import unittest
 
 from SoftLayer.decoration import retry
 from SoftLayer import exceptions
@@ -89,7 +88,3 @@ def raise_unexpected_error():
             raise TypeError('unexpected error')
 
         self.assertRaises(TypeError, raise_unexpected_error)
-
-if __name__ == '__main__':
-
-    unittest.main()
diff --git a/tests/transport_tests.py b/tests/transport_tests.py
index 690882f6..c9e9304c 100644
--- a/tests/transport_tests.py
+++ b/tests/transport_tests.py
@@ -44,7 +44,7 @@ def set_up(self):
         )
         self.response = get_xmlrpc_response()
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_call(self, request):
         request.return_value = self.response
 
@@ -95,7 +95,7 @@ def test_proxy_without_protocol(self):
             warnings.warn("Incorrect Exception raised. Expected a "
                           "SoftLayer.TransportError error")
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_valid_proxy(self, request):
         request.return_value = self.response
         self.transport.proxy = 'http://localhost:3128'
@@ -116,7 +116,7 @@ def test_valid_proxy(self, request):
             cert=None,
             verify=True)
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_identifier(self, request):
         request.return_value = self.response
 
@@ -134,7 +134,7 @@ def test_identifier(self, request):
 <value><int>1234</int></value>
 </member>""", kwargs['data'])
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_filter(self, request):
         request.return_value = self.response
 
@@ -152,7 +152,7 @@ def test_filter(self, request):
 <value><string>^= prefix</string></value>
 </member>""", kwargs['data'])
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_limit_offset(self, request):
         request.return_value = self.response
 
@@ -172,7 +172,7 @@ def test_limit_offset(self, request):
 <value><int>10</int></value>
 </member>""", kwargs['data'])
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_old_mask(self, request):
         request.return_value = self.response
 
@@ -194,7 +194,7 @@ def test_old_mask(self, request):
 </struct></value>
 </member>""", kwargs['data'])
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_mask_call_no_mask_prefix(self, request):
         request.return_value = self.response
 
@@ -210,7 +210,7 @@ def test_mask_call_no_mask_prefix(self, request):
             "<value><string>mask[something.nested]</string></value>",
             kwargs['data'])
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_mask_call_v2(self, request):
         request.return_value = self.response
 
@@ -226,7 +226,7 @@ def test_mask_call_v2(self, request):
             "<value><string>mask[something[nested]]</string></value>",
             kwargs['data'])
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_mask_call_v2_dot(self, request):
         request.return_value = self.response
 
@@ -241,7 +241,7 @@ def test_mask_call_v2_dot(self, request):
         self.assertIn("<value><string>mask.something.nested</string></value>",
                       kwargs['data'])
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_request_exception(self, request):
         # Test Text Error
         e = requests.HTTPError('error')
@@ -257,7 +257,7 @@ def test_request_exception(self, request):
         self.assertRaises(SoftLayer.TransportError, self.transport, req)
 
 
-@mock.patch('requests.request')
+@mock.patch('SoftLayer.transports.requests.Session.request')
 @pytest.mark.parametrize(
     "transport_verify,request_verify,expected",
     [
@@ -313,7 +313,7 @@ def set_up(self):
             endpoint_url='http://something.com',
         )
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_basic(self, request):
         request().content = '[]'
         request().text = '[]'
@@ -340,7 +340,7 @@ def test_basic(self, request):
             proxies=None,
             timeout=None)
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_error(self, request):
         # Test JSON Error
         e = requests.HTTPError('error')
@@ -369,7 +369,7 @@ def test_proxy_without_protocol(self):
             warnings.warn("AssertionError raised instead of a "
                           "SoftLayer.TransportError error")
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_valid_proxy(self, request):
         request().text = '{}'
         self.transport.proxy = 'http://localhost:3128'
@@ -392,7 +392,7 @@ def test_valid_proxy(self, request):
             timeout=mock.ANY,
             headers=mock.ANY)
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_with_id(self, request):
         request().text = '{}'
 
@@ -416,7 +416,7 @@ def test_with_id(self, request):
             proxies=None,
             timeout=None)
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_with_args(self, request):
         request().text = '{}'
 
@@ -440,7 +440,7 @@ def test_with_args(self, request):
             proxies=None,
             timeout=None)
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_with_filter(self, request):
         request().text = '{}'
 
@@ -465,7 +465,7 @@ def test_with_filter(self, request):
             proxies=None,
             timeout=None)
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_with_mask(self, request):
         request().text = '{}'
 
@@ -510,7 +510,7 @@ def test_with_mask(self, request):
             proxies=None,
             timeout=None)
 
-    @mock.patch('requests.request')
+    @mock.patch('SoftLayer.transports.requests.Session.request')
     def test_unknown_error(self, request):
         e = requests.RequestException('error')
         e.response = mock.MagicMock()
@@ -523,3 +523,81 @@ def test_unknown_error(self, request):
         req.method = 'getObject'
 
         self.assertRaises(SoftLayer.TransportError, self.transport, req)
+
+    @mock.patch('SoftLayer.transports.requests.Session.request')
+    def test_with_limits(self, request):
+        request().text = '{}'
+
+        req = transports.Request()
+        req.service = 'SoftLayer_Service'
+        req.method = 'getObject'
+        req.limit = 10
+        req.offset = 10
+
+        resp = self.transport(req)
+
+        self.assertEqual(resp, {})
+        request.assert_called_with(
+            'GET',
+            'http://something.com/SoftLayer_Service/getObject.json',
+            headers=mock.ANY,
+            auth=None,
+            data=None,
+            params={'limit': 10, 'offset': 10},
+            verify=True,
+            cert=None,
+            proxies=None,
+            timeout=None)
+
+    @mock.patch('SoftLayer.transports.requests.Session.request')
+    def test_with_username(self, request):
+        request().text = '{}'
+
+        req = transports.Request()
+        req.service = 'SoftLayer_Service'
+        req.method = 'getObject'
+        req.transport_user = 'Bob'
+        req.transport_password = '123456'
+        auth = requests.auth.HTTPBasicAuth(req.transport_user, req.transport_password)
+
+        resp = self.transport(req)
+
+        self.assertEqual(resp, {})
+        request.assert_called_with(
+            'GET',
+            'http://something.com/SoftLayer_Service/getObject.json',
+            headers=mock.ANY,
+            auth=auth,
+            data=None,
+            params={},
+            verify=True,
+            cert=None,
+            proxies=None,
+            timeout=None)
+
+
+class TestFixtureTransport(testing.TestCase):
+
+    def set_up(self):
+        transport = testing.MockableTransport(SoftLayer.FixtureTransport())
+        self.env.client = SoftLayer.BaseClient(transport=transport)
+
+    def test_base_case(self):
+        result = self.env.client.call('SoftLayer_Account', 'getObject')
+        self.assertEqual(result['accountId'], 1234)
+
+    def test_Import_Service_Error(self):
+        # assertRaises doesn't work here for some reason
+        try:
+            self.env.client.call('FAKE', 'getObject')
+            self.assertTrue(False)
+        except NotImplementedError:
+            self.assertTrue(True)
+
+    def test_Import_Method_Error(self):
+        # assertRaises doesn't work here for some reason
+        try:
+            self.env.client.call('SoftLayer_Account', 'getObject111')
+            self.assertTrue(False)
+        except NotImplementedError:
+            self.assertTrue(True)
diff --git a/tools/test-requirements.txt b/tools/test-requirements.txt
index 2307ed44..fc3f7571 100644
--- a/tools/test-requirements.txt
+++ b/tools/test-requirements.txt
@@ -4,3 +4,4 @@ pytest-cov
 mock
 sphinx
 testtools
+urllib3