From 54692135af8b0a5bdb68a86ceb40178b21a166d3 Mon Sep 17 00:00:00 2001 From: Anthony Monthe Date: Sat, 23 Dec 2017 01:52:36 +0000 Subject: [PATCH 1/4] Added usage of requests.Session --- SoftLayer/transports.py | 63 ++++++++++++++++++++++++++++------------ tests/transport_tests.py | 38 ++++++++++++------------ 2 files changed, 63 insertions(+), 38 deletions(-) diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 32bda03e7..1b40f91aa 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -11,6 +11,8 @@ import time import requests +from requests.adapters import HTTPAdapter +from requests.packages.urllib3.util.retry import Retry from SoftLayer import consts from SoftLayer import exceptions @@ -108,6 +110,19 @@ def __init__(self, endpoint_url=None, timeout=None, proxy=None, user_agent=None, self.user_agent = user_agent or consts.USER_AGENT self.verify = verify + @property + def client(self): + if not hasattr(self, '_client'): + self._client = requests.Session() + self._client.headers.update({ + 'Content-Type': 'application/json', + 'User-Agent': self.user_agent, + }) + retry = Retry(connect=3, backoff_factor=3) + adapter = HTTPAdapter(max_retries=retry) + self._client.mount('https://', adapter) + return self._client + def __call__(self, request): """Makes a SoftLayer API call against the XML-RPC endpoint. @@ -154,13 +169,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) @@ -209,6 +224,19 @@ def __init__(self, endpoint_url=None, timeout=None, proxy=None, user_agent=None, self.user_agent = user_agent or consts.USER_AGENT self.verify = verify + @property + def client(self): + if not hasattr(self, '_client'): + self._client = requests.Session() + self._client.headers.update({ + 'Content-Type': 'application/json', + 'User-Agent': self.user_agent, + }) + retry = Retry(connect=3, backoff_factor=3) + adapter = HTTPAdapter(max_retries=retry) + self._client.mount('https://', adapter) + return self._client + def __call__(self, request): """Makes a SoftLayer API call against the REST endpoint. @@ -217,9 +245,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 +300,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) diff --git a/tests/transport_tests.py b/tests/transport_tests.py index 690882f69..5bd88e771 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): 1234 """, 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): ^= prefix """, 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): 10 """, 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): """, 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): "mask[something.nested]", 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): "mask[something[nested]]", 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("mask.something.nested", 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() From fa69257ba616b8a7e262e2203edf481fd0a7b2b6 Mon Sep 17 00:00:00 2001 From: Anthony Monthe Date: Sat, 23 Dec 2017 01:57:55 +0000 Subject: [PATCH 2/4] DRYed client --- SoftLayer/transports.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 1b40f91aa..d5cb919b0 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -40,6 +40,18 @@ } +def get_session(user_agent): + 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.""" @@ -113,14 +125,7 @@ def __init__(self, endpoint_url=None, timeout=None, proxy=None, user_agent=None, @property def client(self): if not hasattr(self, '_client'): - self._client = requests.Session() - self._client.headers.update({ - 'Content-Type': 'application/json', - 'User-Agent': self.user_agent, - }) - retry = Retry(connect=3, backoff_factor=3) - adapter = HTTPAdapter(max_retries=retry) - self._client.mount('https://', adapter) + self._client = get_session(self.user_agent) return self._client def __call__(self, request): @@ -227,14 +232,7 @@ def __init__(self, endpoint_url=None, timeout=None, proxy=None, user_agent=None, @property def client(self): if not hasattr(self, '_client'): - self._client = requests.Session() - self._client.headers.update({ - 'Content-Type': 'application/json', - 'User-Agent': self.user_agent, - }) - retry = Retry(connect=3, backoff_factor=3) - adapter = HTTPAdapter(max_retries=retry) - self._client.mount('https://', adapter) + self._client = get_session(self.user_agent) return self._client def __call__(self, request): From 43ea8f6e16a213aa107031d3966ea97afa903828 Mon Sep 17 00:00:00 2001 From: Anthony Monthe Date: Sat, 23 Dec 2017 01:59:14 +0000 Subject: [PATCH 3/4] Added to CONTRIBUTORS --- CONTRIBUTORS | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index c0971279a..4de814b58 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -29,3 +29,4 @@ Swapnil Khanapurkar The SoftLayer Developer Network Tim Ariyeh Wissam Elriachy +Anthony Monthe (ZuluPro) From 8a9d858e13f8947885148305a51729b22a8ff305 Mon Sep 17 00:00:00 2001 From: Christopher Gallo Date: Wed, 27 Dec 2017 16:35:48 -0600 Subject: [PATCH 4/4] upgrading requests and urllib3 libraries to better support retries --- SoftLayer/fixtures/SoftLayer_Account.py | 3 +- SoftLayer/transports.py | 20 ++++--- setup.py | 3 +- tests/decoration_tests.py | 5 -- tests/transport_tests.py | 78 +++++++++++++++++++++++++ tools/requirements.txt | 1 + tools/test-requirements.txt | 1 + 7 files changed, 97 insertions(+), 14 deletions(-) diff --git a/SoftLayer/fixtures/SoftLayer_Account.py b/SoftLayer/fixtures/SoftLayer_Account.py index 0f3a0a6a9..4a59000b5 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 d5cb919b0..97d67a5b9 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -12,7 +12,7 @@ import requests from requests.adapters import HTTPAdapter -from requests.packages.urllib3.util.retry import Retry +from urllib3.util.retry import Retry from SoftLayer import consts from SoftLayer import exceptions @@ -41,6 +41,8 @@ def get_session(user_agent): + """Sets up urllib sessions""" + client = requests.Session() client.headers.update({ 'Content-Type': 'application/json', @@ -121,10 +123,13 @@ 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): - if not hasattr(self, '_client'): + """Returns client session object""" + + if self._client is None: self._client = get_session(self.user_agent) return self._client @@ -228,10 +233,13 @@ 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): - if not hasattr(self, '_client'): + """Returns client session object""" + + if self._client is None: self._client = get_session(self.user_agent) return self._client @@ -362,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 fb252243c..23db6ab69 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/tests/decoration_tests.py b/tests/decoration_tests.py index 785d47584..9d230671c 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 5bd88e771..c9e9304c3 100644 --- a/tests/transport_tests.py +++ b/tests/transport_tests.py @@ -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/requirements.txt b/tools/requirements.txt index 6eb1237b6..4663c4302 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 diff --git a/tools/test-requirements.txt b/tools/test-requirements.txt index 2307ed444..fc3f75715 100644 --- a/tools/test-requirements.txt +++ b/tools/test-requirements.txt @@ -4,3 +4,4 @@ pytest-cov mock sphinx testtools +urllib3