From 2a2ca0ff15acc8bafa7f59f1a116dc697c95bcb5 Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Thu, 4 Sep 2014 12:41:19 -0500 Subject: [PATCH 1/3] Fixes test failures due to requests changes --- SoftLayer/tests/functional_tests.py | 6 ++--- SoftLayer/tests/transport_tests.py | 35 +++++++++++++++++++---------- SoftLayer/transports.py | 19 +++++++++------- tox.ini | 3 ++- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/SoftLayer/tests/functional_tests.py b/SoftLayer/tests/functional_tests.py index 025e59ed1..ed70fe877 100644 --- a/SoftLayer/tests/functional_tests.py +++ b/SoftLayer/tests/functional_tests.py @@ -37,10 +37,8 @@ def test_no_hostname(self): # This test will fail if 'notvalidsoftlayer.com' becomes a thing SoftLayer.transports.make_xml_rpc_api_call( 'http://notvalidsoftlayer.com', 'getObject') - except SoftLayer.SoftLayerAPIError as e: - self.assertEqual(e.faultCode, 0) - self.assertIn('not known', e.faultString) - self.assertIn('not known', e.reason) + except Exception as ex: + self.assertIn('not known', str(ex)) else: self.fail('No Exception Raised') diff --git a/SoftLayer/tests/transport_tests.py b/SoftLayer/tests/transport_tests.py index 196795dbc..5284e2bed 100644 --- a/SoftLayer/tests/transport_tests.py +++ b/SoftLayer/tests/transport_tests.py @@ -15,7 +15,8 @@ class TestXmlRpcAPICall(testing.TestCase): def set_up(self): - self.send_content = ''' + self.response = mock.MagicMock() + self.response.content = ''' @@ -26,9 +27,9 @@ def set_up(self): ''' - @mock.patch('SoftLayer.transports.requests.Session.send') - def test_call(self, send): - send().content = self.send_content + @mock.patch('requests.request') + def test_call(self, request): + request.return_value = self.response data = ''' @@ -46,33 +47,43 @@ def test_call(self, send): ''' resp = transports.make_xml_rpc_api_call( 'http://something.com/path/to/resource', 'getObject') - args = send.call_args + args = request.call_args self.assertIsNotNone(args) args, kwargs = args - send.assert_called_with(mock.ANY, proxies=None, timeout=None) + request.assert_called_with('POST', + 'http://something.com/path/to/resource', + headers=None, + proxies=None, + data=data, + timeout=None) self.assertEqual(resp, []) - self.assertEqual(args[0].body, data) def test_proxy_without_protocol(self): + # NOTE(sudorandom): This used to be an instance of requests.HTTPError, + # but something changes in requests to make that no + # longer the case. self.assertRaises( - SoftLayer.TransportError, + Exception, # NOQA transports.make_xml_rpc_api_call, 'http://something.com/path/to/resource', 'getObject', 'localhost:3128') - @mock.patch('SoftLayer.transports.requests.Session.send') - def test_valid_proxy(self, send): - send().content = self.send_content + @mock.patch('requests.request') + def test_valid_proxy(self, request): + request.return_value = self.response transports.make_xml_rpc_api_call( 'http://something.com/path/to/resource', 'getObject', proxy='http://localhost:3128') - send.assert_called_with( + request.assert_called_with( + 'POST', mock.ANY, + headers=None, proxies={'https': 'http://localhost:3128', 'http': 'http://localhost:3128'}, + data=mock.ANY, timeout=None) diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 364c60f24..44fc9812c 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -42,17 +42,16 @@ def make_xml_rpc_api_call(uri, method, args=None, headers=None, payload = utils.xmlrpc_client.dumps(tuple(largs), methodname=method, allow_none=True) - session = requests.Session() - req = requests.Request('POST', uri, data=payload, - headers=http_headers).prepare() LOGGER.debug("=== REQUEST ===") LOGGER.info('POST %s', uri) - LOGGER.debug(req.headers) + LOGGER.debug(http_headers) LOGGER.debug(payload) - response = session.send(req, - timeout=timeout, - proxies=_proxies_dict(proxy)) + response = requests.request('POST', uri, + data=payload, + headers=http_headers, + timeout=timeout, + proxies=_proxies_dict(proxy)) LOGGER.debug("=== RESPONSE ===") LOGGER.debug(response.headers) LOGGER.debug(response.content) @@ -91,14 +90,18 @@ def make_rest_api_call(method, url, :param dict http_headers: HTTP headers to use for the request :param int timeout: number of seconds to use as a timeout """ + LOGGER.debug("=== REQUEST ===") LOGGER.info('%s %s', method, url) + LOGGER.debug(http_headers) try: resp = requests.request(method, url, headers=http_headers, timeout=timeout, proxies=_proxies_dict(proxy)) - resp.raise_for_status() + LOGGER.debug("=== RESPONSE ===") + LOGGER.debug(resp.headers) LOGGER.debug(resp.content) + resp.raise_for_status() if url.endswith('.json'): return json.loads(resp.content) else: diff --git a/tox.ini b/tox.ini index 8e68e57e4..5abdf9e0d 100644 --- a/tox.ini +++ b/tox.ini @@ -21,10 +21,11 @@ deps = hacking pylint commands = - flake8 --max-complexity=36 --statistics \ + flake8 --max-complexity=36 \ --ignore=H401,H402,H404,H405 \ SoftLayer pylint SoftLayer \ + -r n \ # Don't show the long report --ignore=tests,testing \ -d R0903 \ # Too few public methods -d R0914 \ # Too many local variables From 61d62f2b67062ca5aaa804fbe855a3ed9379f04e Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Thu, 4 Sep 2014 12:49:32 -0500 Subject: [PATCH 2/3] Adds notes/references to the requests lib issue --- SoftLayer/tests/functional_tests.py | 3 +++ SoftLayer/tests/transport_tests.py | 2 ++ 2 files changed, 5 insertions(+) diff --git a/SoftLayer/tests/functional_tests.py b/SoftLayer/tests/functional_tests.py index ed70fe877..9f2af984d 100644 --- a/SoftLayer/tests/functional_tests.py +++ b/SoftLayer/tests/functional_tests.py @@ -38,6 +38,9 @@ def test_no_hostname(self): SoftLayer.transports.make_xml_rpc_api_call( 'http://notvalidsoftlayer.com', 'getObject') except Exception as ex: + # NOTE(sudorandom): This used to be an instance of + # SoftLayer.SoftLayerAPIError + # Related issue: https://github.com/kennethreitz/requests/pull/2193 self.assertIn('not known', str(ex)) else: self.fail('No Exception Raised') diff --git a/SoftLayer/tests/transport_tests.py b/SoftLayer/tests/transport_tests.py index 5284e2bed..bacd3c887 100644 --- a/SoftLayer/tests/transport_tests.py +++ b/SoftLayer/tests/transport_tests.py @@ -63,6 +63,8 @@ def test_proxy_without_protocol(self): # NOTE(sudorandom): This used to be an instance of requests.HTTPError, # but something changes in requests to make that no # longer the case. + # Related issue: + # https://github.com/kennethreitz/requests/pull/2193 self.assertRaises( Exception, # NOQA transports.make_xml_rpc_api_call, From a61500e62981c11f9c0403aebe68e12112758699 Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Tue, 9 Sep 2014 15:31:18 -0500 Subject: [PATCH 3/3] Reverts tests because Requests 2.4.1 was released --- SoftLayer/tests/functional_tests.py | 7 +++---- SoftLayer/tests/transport_tests.py | 7 +------ 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/SoftLayer/tests/functional_tests.py b/SoftLayer/tests/functional_tests.py index 9f2af984d..d121f9958 100644 --- a/SoftLayer/tests/functional_tests.py +++ b/SoftLayer/tests/functional_tests.py @@ -37,11 +37,10 @@ def test_no_hostname(self): # This test will fail if 'notvalidsoftlayer.com' becomes a thing SoftLayer.transports.make_xml_rpc_api_call( 'http://notvalidsoftlayer.com', 'getObject') - except Exception as ex: - # NOTE(sudorandom): This used to be an instance of - # SoftLayer.SoftLayerAPIError - # Related issue: https://github.com/kennethreitz/requests/pull/2193 + except SoftLayer.SoftLayerAPIError as ex: self.assertIn('not known', str(ex)) + self.assertIn('not known', ex.faultString) + self.assertEqual(ex.faultCode, 0) else: self.fail('No Exception Raised') diff --git a/SoftLayer/tests/transport_tests.py b/SoftLayer/tests/transport_tests.py index bacd3c887..a0e82f558 100644 --- a/SoftLayer/tests/transport_tests.py +++ b/SoftLayer/tests/transport_tests.py @@ -60,13 +60,8 @@ def test_call(self, request): self.assertEqual(resp, []) def test_proxy_without_protocol(self): - # NOTE(sudorandom): This used to be an instance of requests.HTTPError, - # but something changes in requests to make that no - # longer the case. - # Related issue: - # https://github.com/kennethreitz/requests/pull/2193 self.assertRaises( - Exception, # NOQA + SoftLayer.TransportError, # NOQA transports.make_xml_rpc_api_call, 'http://something.com/path/to/resource', 'getObject',