From 8deeb4c160c358c273448493eb9400467c518955 Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Mon, 22 Sep 2014 17:05:12 -0500 Subject: [PATCH 1/7] Adds generic SoftLayer request object This allows both transports to (mostly) use this one object to define the specifications of an API call. --- SoftLayer/API.py | 64 ++--- SoftLayer/auth.py | 16 +- SoftLayer/managers/metadata.py | 39 ++- SoftLayer/tests/api_tests.py | 177 ++++--------- SoftLayer/tests/auth_tests.py | 37 ++- SoftLayer/tests/deprecated_tests.py | 16 +- SoftLayer/tests/functional_tests.py | 10 +- SoftLayer/tests/managers/metadata_tests.py | 151 ++++++----- SoftLayer/tests/transport_tests.py | 286 +++++++++++++++++---- SoftLayer/transports.py | 170 +++++++++--- setup.cfg | 2 +- 11 files changed, 575 insertions(+), 393 deletions(-) diff --git a/SoftLayer/API.py b/SoftLayer/API.py index e1956c92a..cab5e6921 100644 --- a/SoftLayer/API.py +++ b/SoftLayer/API.py @@ -145,24 +145,6 @@ def call(self, service, method, *args, **kwargs): if not service.startswith(self._prefix): service = self._prefix + service - headers = kwargs.get('headers', {}) - - if kwargs.get('id') is not None: - headers[service + 'InitParameters'] = {'id': kwargs.get('id')} - - if kwargs.get('mask') is not None: - headers.update(self.__format_object_mask(kwargs.get('mask'), - service)) - - if kwargs.get('filter') is not None: - headers['%sObjectFilter' % service] = kwargs.get('filter') - - if kwargs.get('limit'): - headers['resultLimit'] = { - 'limit': kwargs.get('limit'), - 'offset': kwargs.get('offset', 0), - } - http_headers = { 'User-Agent': self.user_agent or consts.USER_AGENT, 'Content-Type': 'application/xml', @@ -175,13 +157,19 @@ def call(self, service, method, *args, **kwargs): if kwargs.get('raw_headers'): http_headers.update(kwargs.get('raw_headers')) - uri = '/'.join([self.endpoint_url, service]) - options = { - 'headers': headers, - 'http_headers': http_headers, - 'timeout': self.timeout, - 'proxy': self.proxy, - } + request = transports.Request() + request.endpoint = self.endpoint_url + request.service = service + request.method = method + request.args = args + request.transport_headers = http_headers + request.timeout = self. timeout + request.proxy = self.proxy + request.identifier = kwargs.get('id') + request.mask = kwargs.get('mask') + request.filter = kwargs.get('filter') + request.limit = kwargs.get('limit') + request.offset = kwargs.get('offset') if self.auth: extra_headers = self.auth.get_headers() @@ -189,12 +177,11 @@ def call(self, service, method, *args, **kwargs): warnings.warn("auth.get_headers() is deprecated and will be " "removed in the next major version", DeprecationWarning) - headers.update(extra_headers) + request.headers.update(extra_headers) - options = self.auth.get_options(options) + request = self.auth.get_request(request) - return transports.make_xml_rpc_api_call(uri, method, args, - **options) + return transports.make_xml_rpc_api_call(request) __call__ = call @@ -249,25 +236,6 @@ def iter_call(self, service, method, if len(results) < chunk: break - def __format_object_mask(self, objectmask, service): - """Format new and old style object masks into proper headers. - - :param objectmask: a string- or dict-based object mask - :param service: a SoftLayer API service name - - """ - if isinstance(objectmask, dict): - mheader = '%sObjectMask' % service - else: - mheader = self._prefix + 'ObjectMask' - - objectmask = objectmask.strip() - if (not objectmask.startswith('mask') - and not objectmask.startswith('[')): - objectmask = "mask[%s]" % objectmask - - return {mheader: {'mask': objectmask}} - def __repr__(self): return "" % (self.endpoint_url, self.auth) diff --git a/SoftLayer/auth.py b/SoftLayer/auth.py index 931042e72..c2a1435a4 100644 --- a/SoftLayer/auth.py +++ b/SoftLayer/auth.py @@ -11,13 +11,13 @@ class AuthenticationBase(object): """A base authentication class intended to be overridden.""" - def get_options(self, options): + def get_request(self, request): """Receives request options and returns request options. :param options dict: dictionary of request options """ - return options + return request def get_headers(self): """Return a dictionary of headers to be inserted for authentication. @@ -39,14 +39,14 @@ def __init__(self, user_id, auth_token): self.user_id = user_id self.auth_token = auth_token - def get_options(self, options): + def get_request(self, request): """Sets token-based auth headers.""" - options['headers']['authenticate'] = { + request.headers['authenticate'] = { 'complexType': 'PortalLoginToken', 'userId': self.user_id, 'authToken': self.auth_token, } - return options + return request def __repr__(self): return "" % (self.user_id, self.auth_token) @@ -62,13 +62,13 @@ def __init__(self, username, api_key): self.username = username self.api_key = api_key - def get_options(self, options): + def get_request(self, request): """Sets token-based auth headers.""" - options['headers']['authenticate'] = { + request.headers['authenticate'] = { 'username': self.username, 'apiKey': self.api_key, } - return options + return request def __repr__(self): return "" % (self.username) diff --git a/SoftLayer/managers/metadata.py b/SoftLayer/managers/metadata.py index dda795812..6d6b47b2a 100644 --- a/SoftLayer/managers/metadata.py +++ b/SoftLayer/managers/metadata.py @@ -58,22 +58,6 @@ def __init__(self, client=None, timeout=5): self.timeout = timeout self.client = client - def make_request(self, path): - """ Make a request against the metadata service - - :param string path: path to the specific metadata resource - """ - url = '/'.join([self.url, 'SoftLayer_Resource_Metadata', path]) - try: - return transports.make_rest_api_call( - 'GET', url, - http_headers={'User-Agent': consts.USER_AGENT}, - timeout=self.timeout) - except exceptions.SoftLayerAPIError as ex: - if ex.faultCode == 404: - return None - raise ex - def get(self, name, param=None): """ Retreive a metadata attribute @@ -85,19 +69,30 @@ def get(self, name, param=None): raise exceptions.SoftLayerError('Unknown metadata attribute.') call_details = self.attribs[name] - extension = '.json' + extension = 'json' if self.attribs[name]['call'] == 'UserMetadata': - extension = '.txt' + extension = 'txt' if call_details.get('param_req'): if not param: raise exceptions.SoftLayerError( 'Parameter required to get this attribute.') - path = "%s/%s%s" % (self.attribs[name]['call'], param, extension) - else: - path = "%s%s" % (self.attribs[name]['call'], extension) - return self.make_request(path) + request = transports.Request() + request.endpoint = self.url + request.service = 'SoftLayer_Resource_Metadata' + request.method = self.attribs[name]['call'] + request.transport_headers = {'User-Agent': consts.USER_AGENT} + request.timeout = self.timeout + request.identifier = param + + try: + return transports.make_rest_api_call(request, + extension=extension) + except exceptions.SoftLayerAPIError as ex: + if ex.faultCode == 404: + return None + raise ex def _get_network(self, kind, router=True, vlans=True, vlan_ids=True): """ Wrapper for getting details about networks diff --git a/SoftLayer/tests/api_tests.py b/SoftLayer/tests/api_tests.py index 8260f4e62..c64c33d4e 100644 --- a/SoftLayer/tests/api_tests.py +++ b/SoftLayer/tests/api_tests.py @@ -8,9 +8,11 @@ import SoftLayer import SoftLayer.API -from SoftLayer import consts from SoftLayer import testing +TEST_AUTH_HEADERS = { + 'authenticate': {'apiKey': 'issurelywrong', 'username': 'doesnotexist'}} + class Inititialization(testing.TestCase): def test_init(self): @@ -79,109 +81,49 @@ def set_up(self): @mock.patch('SoftLayer.transports.make_xml_rpc_api_call') def test_simple_call(self, make_xml_rpc_api_call): - self.client['SERVICE'].METHOD() - make_xml_rpc_api_call.assert_called_with( - 'ENDPOINT/SoftLayer_SERVICE', 'METHOD', (), - headers={ - 'authenticate': { - 'username': 'doesnotexist', 'apiKey': 'issurelywrong'}}, - proxy=None, - timeout=None, - http_headers={ - 'Content-Type': 'application/xml', - 'User-Agent': consts.USER_AGENT, - 'Accept': '*/*', - 'Accept-Encoding': 'gzip, deflate, compress', - }) + make_xml_rpc_api_call.return_value = {"test": "result"} + resp = self.client['SERVICE'].METHOD() + + self.assertEqual(resp, {"test": "result"}) + + (request,), kwargs = make_xml_rpc_api_call.call_args + self.assertEqual(request.endpoint, self.client.endpoint_url) + self.assertEqual(request.service, 'SoftLayer_SERVICE') + self.assertEqual(request.method, 'METHOD') + self.assertEqual(request.mask, None) + self.assertEqual(request.filter, None) + self.assertEqual(request.identifier, None) + self.assertEqual(request.args, tuple()) + self.assertEqual(request.limit, None) + self.assertEqual(request.offset, None) + self.assertEqual(request.headers, TEST_AUTH_HEADERS) @mock.patch('SoftLayer.transports.make_xml_rpc_api_call') def test_complex(self, make_xml_rpc_api_call): - self.client['SERVICE'].METHOD( + make_xml_rpc_api_call.return_value = {"test": "result"} + resp = self.client['SERVICE'].METHOD( 1234, id=5678, mask={'object': {'attribute': ''}}, raw_headers={'RAW': 'HEADER'}, filter={ - 'TYPE': {'obj': {'attribute': {'operation': '^= prefix'}}}}, + 'TYPE': {'attribute': {'operation': '^= prefix'}}}, limit=9, offset=10) - make_xml_rpc_api_call.assert_called_with( - 'ENDPOINT/SoftLayer_SERVICE', 'METHOD', (1234, ), - headers={ - 'SoftLayer_SERVICEObjectMask': { - 'mask': {'object': {'attribute': ''}}}, - 'SoftLayer_SERVICEObjectFilter': { - 'TYPE': { - 'obj': {'attribute': {'operation': '^= prefix'}}}}, - 'authenticate': { - 'username': 'doesnotexist', 'apiKey': 'issurelywrong'}, - 'SoftLayer_SERVICEInitParameters': {'id': 5678}, - 'resultLimit': {'limit': 9, 'offset': 10}}, - proxy=None, - timeout=None, - http_headers={ - 'RAW': 'HEADER', - 'Content-Type': 'application/xml', - 'User-Agent': consts.USER_AGENT, - 'Accept': '*/*', - 'Accept-Encoding': 'gzip, deflate, compress', - }) - - @mock.patch('SoftLayer.transports.make_xml_rpc_api_call') - def test_mask_call_v2(self, make_xml_rpc_api_call): - self.client['SERVICE'].METHOD( - mask="mask[something[nested]]") - make_xml_rpc_api_call.assert_called_with( - 'ENDPOINT/SoftLayer_SERVICE', 'METHOD', (), - headers={ - 'authenticate': { - 'username': 'doesnotexist', 'apiKey': 'issurelywrong'}, - 'SoftLayer_ObjectMask': {'mask': 'mask[something[nested]]'}}, - proxy=None, - timeout=None, - http_headers={ - 'Content-Type': 'application/xml', - 'User-Agent': consts.USER_AGENT, - 'Accept': '*/*', - 'Accept-Encoding': 'gzip, deflate, compress', - }) - - @mock.patch('SoftLayer.transports.make_xml_rpc_api_call') - def test_mask_call_v2_dot(self, make_xml_rpc_api_call): - self.client['SERVICE'].METHOD( - mask="mask.something.nested") - make_xml_rpc_api_call.assert_called_with( - 'ENDPOINT/SoftLayer_SERVICE', 'METHOD', (), - headers={ - 'authenticate': { - 'username': 'doesnotexist', 'apiKey': 'issurelywrong'}, - 'SoftLayer_ObjectMask': {'mask': 'mask.something.nested'}}, - proxy=None, - timeout=None, - http_headers={ - 'Content-Type': 'application/xml', - 'User-Agent': consts.USER_AGENT, - 'Accept': '*/*', - 'Accept-Encoding': 'gzip, deflate, compress', - }) - - @mock.patch('SoftLayer.transports.make_xml_rpc_api_call') - def test_mask_call_no_mask_prefix(self, make_xml_rpc_api_call): - self.client['SERVICE'].METHOD(mask="something.nested") - make_xml_rpc_api_call.assert_called_with( - 'ENDPOINT/SoftLayer_SERVICE', 'METHOD', (), - headers={ - 'authenticate': { - 'username': 'doesnotexist', 'apiKey': 'issurelywrong'}, - 'SoftLayer_ObjectMask': {'mask': 'mask[something.nested]'}}, - proxy=None, - timeout=None, - http_headers={ - 'Content-Type': 'application/xml', - 'User-Agent': consts.USER_AGENT, - 'Accept': '*/*', - 'Accept-Encoding': 'gzip, deflate, compress', - }) + self.assertEqual(resp, {"test": "result"}) + + (request,), kwargs = make_xml_rpc_api_call.call_args + self.assertEqual(request.endpoint, self.client.endpoint_url) + self.assertEqual(request.service, 'SoftLayer_SERVICE') + self.assertEqual(request.method, 'METHOD') + self.assertEqual(request.mask, {'object': {'attribute': ''}}) + self.assertEqual(request.filter, + {'TYPE': {'attribute': {'operation': '^= prefix'}}}) + self.assertEqual(request.identifier, 5678) + self.assertEqual(request.args, (1234,)) + self.assertEqual(request.limit, 9) + self.assertEqual(request.offset, 10) + self.assertEqual(request.headers, TEST_AUTH_HEADERS) @mock.patch('SoftLayer.API.Client.iter_call') def test_iterate(self, _iter_call): @@ -250,8 +192,8 @@ def test_iter_call(self, _call): # Chunk size of 0 is invalid self.assertRaises( AttributeError, - lambda: list(self.client.iter_call( - 'SERVICE', 'METHOD', iter=True, chunk=0))) + lambda: list(self.client.iter_call('SERVICE', 'METHOD', + iter=True, chunk=0))) def test_call_invalid_arguments(self): self.assertRaises( @@ -261,30 +203,19 @@ def test_call_invalid_arguments(self): @mock.patch('SoftLayer.transports.make_xml_rpc_api_call') def test_call_compression_disabled(self, make_xml_rpc_api_call): self.client['SERVICE'].METHOD(compress=False) - make_xml_rpc_api_call.assert_called_with( - 'ENDPOINT/SoftLayer_SERVICE', 'METHOD', (), - headers=mock.ANY, - proxy=None, - timeout=None, - http_headers={ - 'Content-Type': 'application/xml', - 'User-Agent': consts.USER_AGENT, - }) + + (request,), kwargs = make_xml_rpc_api_call.call_args + self.assertNotIn('Accept-Encoding', request.transport_headers) + self.assertNotIn('Accept', request.transport_headers) @mock.patch('SoftLayer.transports.make_xml_rpc_api_call') def test_call_compression_enabled(self, make_xml_rpc_api_call): self.client['SERVICE'].METHOD(compress=True) - make_xml_rpc_api_call.assert_called_with( - 'ENDPOINT/SoftLayer_SERVICE', 'METHOD', (), - headers=mock.ANY, - proxy=None, - timeout=None, - http_headers={ - 'Content-Type': 'application/xml', - 'User-Agent': consts.USER_AGENT, - 'Accept': '*/*', - 'Accept-Encoding': 'gzip, deflate, compress', - }) + + (request,), kwargs = make_xml_rpc_api_call.call_args + headers = request.transport_headers + self.assertEqual(headers['Accept-Encoding'], 'gzip, deflate, compress') + self.assertEqual(headers['Accept'], '*/*') @mock.patch('SoftLayer.transports.make_xml_rpc_api_call') def test_call_compression_override(self, make_xml_rpc_api_call): @@ -292,16 +223,10 @@ def test_call_compression_override(self, make_xml_rpc_api_call): self.client['SERVICE'].METHOD( compress=False, raw_headers={'Accept-Encoding': 'gzip'}) - make_xml_rpc_api_call.assert_called_with( - 'ENDPOINT/SoftLayer_SERVICE', 'METHOD', (), - headers=mock.ANY, - proxy=None, - timeout=None, - http_headers={ - 'Content-Type': 'application/xml', - 'User-Agent': consts.USER_AGENT, - 'Accept-Encoding': 'gzip', - }) + + (request,), kwargs = make_xml_rpc_api_call.call_args + self.assertEqual(request.transport_headers['Accept-Encoding'], 'gzip') + self.assertNotIn('Accept', request.transport_headers) class APITimedClient(testing.TestCase): diff --git a/SoftLayer/tests/auth_tests.py b/SoftLayer/tests/auth_tests.py index 45405c7e8..240af3a99 100644 --- a/SoftLayer/tests/auth_tests.py +++ b/SoftLayer/tests/auth_tests.py @@ -6,12 +6,13 @@ """ from SoftLayer import auth from SoftLayer import testing +from SoftLayer import transports class TestAuthenticationBase(testing.TestCase): - def test_get_options(self): + def test_get_request(self): auth_base = auth.AuthenticationBase() - self.assertEqual(auth_base.get_options({}), {}) + self.assertEqual(auth_base.get_request({}), {}) self.assertEqual(auth_base.get_headers(), {}) @@ -23,14 +24,13 @@ def test_attribs(self): self.assertEqual(self.auth.username, 'USERNAME') self.assertEqual(self.auth.api_key, 'APIKEY') - def test_get_options(self): - headers = {'headers': {}} - self.assertEqual(self.auth.get_options(headers), { - 'headers': { - 'authenticate': { - 'username': 'USERNAME', - 'apiKey': 'APIKEY', - } + def test_get_request(self): + req = transports.Request() + authed_req = self.auth.get_request(req) + self.assertEqual(authed_req.headers, { + 'authenticate': { + 'username': 'USERNAME', + 'apiKey': 'APIKEY', } }) @@ -48,15 +48,14 @@ def test_attribs(self): self.assertEqual(self.auth.user_id, 12345) self.assertEqual(self.auth.auth_token, 'TOKEN') - def test_get_options(self): - headers = {'headers': {}} - self.assertEqual(self.auth.get_options(headers), { - 'headers': { - 'authenticate': { - 'complexType': 'PortalLoginToken', - 'userId': 12345, - 'authToken': 'TOKEN', - } + def test_get_request(self): + req = transports.Request() + authed_req = self.auth.get_request(req) + self.assertEqual(authed_req.headers, { + 'authenticate': { + 'complexType': 'PortalLoginToken', + 'userId': 12345, + 'authToken': 'TOKEN', } }) diff --git a/SoftLayer/tests/deprecated_tests.py b/SoftLayer/tests/deprecated_tests.py index 207d8e0ed..f4918c91f 100644 --- a/SoftLayer/tests/deprecated_tests.py +++ b/SoftLayer/tests/deprecated_tests.py @@ -30,14 +30,14 @@ def test_simple_call(self, make_xml_rpc_api_call): # Cause all warnings to always be triggered. warnings.simplefilter("always") - self.client['SERVICE'].METHOD() - - make_xml_rpc_api_call.assert_called_with( - mock.ANY, mock.ANY, mock.ANY, - headers={'deprecated': 'header'}, - proxy=mock.ANY, - timeout=mock.ANY, - http_headers=mock.ANY) + make_xml_rpc_api_call.return_value = {"test": "result"} + resp = self.client['SERVICE'].METHOD() + + self.assertEqual(resp, {"test": "result"}) + + (request,), kwargs = make_xml_rpc_api_call.call_args + self.assertEqual(request.headers, {'deprecated': 'header'}) + self.assertEqual(len(w), 1) self.assertEqual(w[0].category, DeprecationWarning) self.assertIn("deprecated", str(w[0].message)) diff --git a/SoftLayer/tests/functional_tests.py b/SoftLayer/tests/functional_tests.py index d121f9958..18408254c 100644 --- a/SoftLayer/tests/functional_tests.py +++ b/SoftLayer/tests/functional_tests.py @@ -8,6 +8,7 @@ import SoftLayer from SoftLayer import testing +from SoftLayer import transports def get_creds(): @@ -34,9 +35,14 @@ def test_failed_auth(self): def test_no_hostname(self): try: + request = transports.Request() + request.endpoint = 'http://notvalidsoftlayer.com' + request.service = 'SoftLayer_Account' + request.method = 'getObject' + request.id = 1234 + # This test will fail if 'notvalidsoftlayer.com' becomes a thing - SoftLayer.transports.make_xml_rpc_api_call( - 'http://notvalidsoftlayer.com', 'getObject') + transports.make_xml_rpc_api_call(request) except SoftLayer.SoftLayerAPIError as ex: self.assertIn('not known', str(ex)) self.assertIn('not known', ex.faultString) diff --git a/SoftLayer/tests/managers/metadata_tests.py b/SoftLayer/tests/managers/metadata_tests.py index 363982c26..93681c052 100644 --- a/SoftLayer/tests/managers/metadata_tests.py +++ b/SoftLayer/tests/managers/metadata_tests.py @@ -15,49 +15,100 @@ class MetadataTests(testing.TestCase): def set_up(self): self.metadata = SoftLayer.MetadataManager() - self.make_request = mock.MagicMock() - self.metadata.make_request = self.make_request - - def test_no_param(self): - self.make_request.return_value = 'dal01' - r = self.metadata.get('datacenter') - self.make_request.assert_called_with("Datacenter.json") - self.assertEqual('dal01', r) - - def test_w_param(self): - self.make_request.return_value = [123] - r = self.metadata.get('vlans', '1:2:3:4:5') - self.make_request.assert_called_with("Vlans/1:2:3:4:5.json") - self.assertEqual([123], r) - - def test_user_data(self): - self.make_request.return_value = 'user_data' - r = self.metadata.get('user_data') - self.make_request.assert_called_with("UserMetadata.txt") - self.assertEqual('user_data', r) - - def test_return_none(self): - self.make_request.return_value = None - r = self.metadata.get('datacenter') - self.make_request.assert_called_with("Datacenter.json") - self.assertEqual(None, r) - - def test_w_param_error(self): + + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_get(self, make_request): + make_request.return_value = 'dal01' + resp = self.metadata.get('datacenter') + + self.assertEqual('dal01', resp) + + (request, ), kwargs = make_request.call_args + + self.assertEqual(request.endpoint, self.metadata.url) + self.assertEqual(request.service, 'SoftLayer_Resource_Metadata') + self.assertEqual(request.method, 'Datacenter') + self.assertEqual(request.transport_headers, + {'User-Agent': consts.USER_AGENT}) + self.assertEqual(request.timeout, self.metadata.timeout) + self.assertEqual(request.identifier, None) + self.assertEqual(kwargs['extension'], 'json') + + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_no_param(self, make_request): + make_request.return_value = 'dal01' + + resp = self.metadata.get('datacenter') + + self.assertEqual('dal01', resp) + (request, ), kwargs = make_request.call_args + self.assertEqual(request.method, 'Datacenter') + self.assertEqual(request.identifier, None) + + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_w_param(self, make_request): + make_request.return_value = [123] + resp = self.metadata.get('vlans', '1:2:3:4:5') + + self.assertEqual([123], resp) + (request, ), kwargs = make_request.call_args + self.assertEqual(request.method, 'Vlans') + self.assertEqual(request.identifier, '1:2:3:4:5') + + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_user_data(self, make_request): + make_request.return_value = 'user_data' + resp = self.metadata.get('user_data') + + self.assertEqual('user_data', resp) + (request, ), kwargs = make_request.call_args + self.assertEqual(request.method, 'UserMetadata') + self.assertEqual(request.identifier, None) + self.assertEqual(kwargs['extension'], 'txt') + + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_return_none(self, make_request): + make_request.return_value = None + resp = self.metadata.get('datacenter') + + self.assertEqual(None, resp) + + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_404(self, make_request): + make_request.side_effect = SoftLayer.SoftLayerAPIError(404, + 'Not Found') + resp = self.metadata.get('user_data') + + self.assertEqual(None, resp) + + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_error(self, make_request): + exception = SoftLayer.SoftLayerAPIError(500, 'Error') + make_request.side_effect = exception + + self.assertRaises(SoftLayer.SoftLayerAPIError, + self.metadata.get, 'user_data') + + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_w_param_error(self, make_request): self.assertRaises(SoftLayer.SoftLayerError, self.metadata.get, 'vlans') - def test_not_exists(self): + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_not_exists(self, make_request): self.assertRaises(SoftLayer.SoftLayerError, self.metadata.get, 'something') - def test_networks_not_exist(self): - self.make_request.return_value = [] + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_networks_not_exist(self, make_request): + make_request.return_value = [] r = self.metadata.public_network() self.assertEqual({'mac_addresses': []}, r) - def test_networks(self): + @mock.patch('SoftLayer.transports.make_rest_api_call') + def test_networks(self, make_request): resp = ['list', 'of', 'stuff'] - self.make_request.return_value = resp + make_request.return_value = resp r = self.metadata.public_network() self.assertEqual({ 'vlan_ids': resp, @@ -73,37 +124,3 @@ def test_networks(self): 'vlans': resp, 'mac_addresses': resp }, r) - - -class MetadataTestsMakeRequest(testing.TestCase): - - def set_up(self): - self.metadata = SoftLayer.MetadataManager() - self.url = '/'.join([ - consts.API_PRIVATE_ENDPOINT_REST.rstrip('/'), - 'SoftLayer_Resource_Metadata', - 'something.json']) - - @mock.patch('SoftLayer.transports.make_rest_api_call') - def test_basic(self, make_api_call): - r = self.metadata.make_request('something.json') - make_api_call.assert_called_with( - 'GET', self.url, - timeout=5, - http_headers={'User-Agent': consts.USER_AGENT}) - self.assertEqual(make_api_call(), r) - - @mock.patch('SoftLayer.transports.make_rest_api_call') - def test_raise_error(self, make_api_call): - make_api_call.side_effect = SoftLayer.SoftLayerAPIError( - 'faultCode', 'faultString') - self.assertRaises( - SoftLayer.SoftLayerAPIError, - self.metadata.make_request, 'something.json') - - @mock.patch('SoftLayer.transports.make_rest_api_call') - def test_raise_404_error(self, make_api_call): - make_api_call.side_effect = SoftLayer.SoftLayerAPIError(404, - 'faultString') - r = self.metadata.make_request('something.json') - self.assertEqual(r, None) diff --git a/SoftLayer/tests/transport_tests.py b/SoftLayer/tests/transport_tests.py index e65ca6595..8d02b0236 100644 --- a/SoftLayer/tests/transport_tests.py +++ b/SoftLayer/tests/transport_tests.py @@ -45,14 +45,15 @@ def test_call(self, request): ''' - resp = transports.make_xml_rpc_api_call( - 'http://something.com/path/to/resource', 'getObject') - args = request.call_args - self.assertIsNotNone(args) - args, kwargs = args + + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'getObject' + resp = transports.make_xml_rpc_api_call(req) request.assert_called_with('POST', - 'http://something.com/path/to/resource', + 'http://something.com/SoftLayer_Service', headers=None, proxies=None, data=data, @@ -62,20 +63,26 @@ def test_call(self, request): self.assertEqual(resp, []) def test_proxy_without_protocol(self): - self.assertRaises( - SoftLayer.TransportError, # NOQA - transports.make_xml_rpc_api_call, - 'http://something.com/path/to/resource', - 'getObject', - 'localhost:3128') + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'Resource' + req.proxy = 'localhost:3128' + + self.assertRaises(SoftLayer.TransportError, + transports.make_xml_rpc_api_call, req) @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') + + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'Resource' + req.proxy = 'http://localhost:3128' + transports.make_xml_rpc_api_call(req) + request.assert_called_with( 'POST', mock.ANY, @@ -87,17 +94,163 @@ def test_valid_proxy(self, request): cert=None, verify=True) + @mock.patch('requests.request') + def test_identifier(self, request): + request.return_value = self.response + + req = transports.Request() + req.endpoint = "http://something.com" + req.service = "SoftLayer_Service" + req.method = "getObject" + req.identifier = 1234 + transports.make_xml_rpc_api_call(req) + + args, kwargs = request.call_args + self.assertIn( + """ +id +1234 +""", kwargs['data']) + + @mock.patch('requests.request') + def test_filter(self, request): + request.return_value = self.response + + req = transports.Request() + req.endpoint = "http://something.com" + req.service = "SoftLayer_Service" + req.method = "getObject" + req.filter = {'TYPE': {'attribute': {'operation': '^= prefix'}}} + transports.make_xml_rpc_api_call(req) + + args, kwargs = request.call_args + self.assertIn( + """ +operation +^= prefix +""", kwargs['data']) + + @mock.patch('requests.request') + def test_limit_offset(self, request): + request.return_value = self.response + + req = transports.Request() + req.endpoint = "http://something.com" + req.service = "SoftLayer_Service" + req.method = "getObject" + req.limit = 10 + transports.make_xml_rpc_api_call(req) + + args, kwargs = request.call_args + self.assertIn(""" +resultLimit + +""", kwargs['data']) + self.assertIn("""limit +10 +""", kwargs['data']) + + @mock.patch('requests.request') + def test_old_mask(self, request): + request.return_value = self.response + + req = transports.Request() + req.endpoint = "http://something.com" + req.service = "SoftLayer_Service" + req.method = "getObject" + req.mask = {"something": "nested"} + transports.make_xml_rpc_api_call(req) + + args, kwargs = request.call_args + self.assertIn(""" +mask + + +something +nested + + +""", kwargs['data']) + + @mock.patch('requests.request') + def test_mask_call_no_mask_prefix(self, request): + request.return_value = self.response + + req = transports.Request() + req.endpoint = "http://something.com" + req.service = "SoftLayer_Service" + req.method = "getObject" + req.mask = "something.nested" + transports.make_xml_rpc_api_call(req) + + args, kwargs = request.call_args + self.assertIn( + "mask[something.nested]", + kwargs['data']) + + @mock.patch('requests.request') + def test_mask_call_v2(self, request): + request.return_value = self.response + + req = transports.Request() + req.endpoint = "http://something.com" + req.service = "SoftLayer_Service" + req.method = "getObject" + req.mask = "mask[something[nested]]" + transports.make_xml_rpc_api_call(req) + + args, kwargs = request.call_args + self.assertIn( + "mask[something[nested]]", + kwargs['data']) + + @mock.patch('requests.request') + def test_mask_call_v2_dot(self, request): + request.return_value = self.response + + req = transports.Request() + req.endpoint = "http://something.com" + req.service = "SoftLayer_Service" + req.method = "getObject" + req.mask = "mask.something.nested" + transports.make_xml_rpc_api_call(req) + + args, kwargs = request.call_args + self.assertIn("mask.something.nested", + kwargs['data']) + + @mock.patch('requests.request') + def test_request_exception(self, request): + # Test Text Error + e = requests.HTTPError('error') + e.response = mock.MagicMock() + e.response.status_code = 404 + e.response.content = 'Error Code' + request().raise_for_status.side_effect = e + + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'getObject' + + self.assertRaises(SoftLayer.TransportError, + transports.make_xml_rpc_api_call, req) + class TestRestAPICall(testing.TestCase): - @mock.patch('SoftLayer.transports.requests.request') + @mock.patch('requests.request') def test_json(self, request): request().content = '{}' - resp = transports.make_rest_api_call( - 'GET', 'http://something.com/path/to/resource.json') + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'Resource' + + resp = transports.make_rest_api_call(req, extension='json') self.assertEqual(resp, {}) request.assert_called_with( - 'GET', 'http://something.com/path/to/resource.json', + 'GET', 'http://something.com/SoftLayer_Service/Resource.json', headers=None, proxies=None, timeout=None) @@ -114,39 +267,71 @@ def test_json(self, request): self.assertRaises( SoftLayer.SoftLayerAPIError, - transports.make_rest_api_call, - 'GET', - 'http://something.com/path/to/resource.json') + transports.make_rest_api_call, req, extension='json') def test_proxy_without_protocol(self): - self.assertRaises( - SoftLayer.TransportError, - transports.make_rest_api_call, - 'GET' - 'http://something.com/path/to/resource.txt', - 'localhost:3128') + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'Resource' + req.proxy = 'localhost:3128' + + self.assertRaises(SoftLayer.TransportError, + transports.make_rest_api_call, req) - @mock.patch('SoftLayer.transports.requests.request') + @mock.patch('requests.request') def test_valid_proxy(self, request): - transports.make_rest_api_call( - 'GET', - 'http://something.com/path/to/resource.txt', - proxy='http://localhost:3128') + request().content = '{}' + + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'Resource' + req.proxy = 'http://localhost:3128' + + transports.make_rest_api_call(req) request.assert_called_with( - 'GET', 'http://something.com/path/to/resource.txt', - headers=mock.ANY, + 'GET', 'http://something.com/SoftLayer_Service/Resource.json', proxies={'https': 'http://localhost:3128', 'http': 'http://localhost:3128'}, + timeout=mock.ANY, + headers=mock.ANY) + + @mock.patch('requests.request') + def test_with_id(self, request): + request().content = '{}' + + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'getObject' + req.identifier = 2 + + resp = transports.make_rest_api_call(req) + + self.assertEqual(resp, {}) + request.assert_called_with( + 'GET', + 'http://something.com/SoftLayer_Service/getObject/2.json', + headers=None, + proxies=None, timeout=None) - @mock.patch('SoftLayer.transports.requests.request') + @mock.patch('requests.request') def test_text(self, request): + request().content = 'content' request().text = 'content' - resp = transports.make_rest_api_call( - 'GET', 'http://something.com/path/to/resource.txt') + + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'Resource' + + resp = transports.make_rest_api_call(req, extension='txt') self.assertEqual(resp, 'content') request.assert_called_with( - 'GET', 'http://something.com/path/to/resource.txt', + 'GET', + 'http://something.com/SoftLayer_Service/Resource.txt', headers=None, proxies=None, timeout=None) @@ -158,13 +343,10 @@ def test_text(self, request): e.response.content = 'Error Code' request().raise_for_status.side_effect = e - self.assertRaises( - SoftLayer.SoftLayerAPIError, - transports.make_rest_api_call, - 'GET', - 'http://something.com/path/to/resource.txt') + self.assertRaises(SoftLayer.SoftLayerAPIError, + transports.make_rest_api_call, req, extension='txt') - @mock.patch('SoftLayer.transports.requests.request') + @mock.patch('requests.request') def test_unknown_error(self, request): e = requests.RequestException('error') e.response = mock.MagicMock() @@ -172,8 +354,10 @@ def test_unknown_error(self, request): e.response.content = 'Error Code' request().raise_for_status.side_effect = e - self.assertRaises( - SoftLayer.TransportError, - transports.make_rest_api_call, - 'GET', - 'http://something.com/path/to/resource.txt') + req = transports.Request() + req.endpoint = 'http://something.com' + req.service = 'SoftLayer_Service' + req.method = 'getObject' + + self.assertRaises(SoftLayer.TransportError, + transports.make_rest_api_call, req) diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 332ae9a98..76a0ea6f2 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -14,49 +14,104 @@ import requests LOGGER = logging.getLogger(__name__) +# transports.Request does have a lot of instance attributes. :( +# pylint: disable=too-many-instance-attributes -def _proxies_dict(proxy): - """Makes a dict appropriate to pass to requests.""" - if not proxy: - return None - return {'http': proxy, 'https': proxy} +class Request(object): + """Transport request object.""" + + def __init__(self): + #: The SoftLayer endpoint address. + self.endpoint = None + + #: Service name. + self.service = None + + #: Method name. + self.method = None + + #: RPC Arguments. + self.args = [] + + #: Transport headers. + self.headers = {} + + #: Transport headers. + self.transport_headers = None + #: Integer timeout. + self.timeout = None -def make_xml_rpc_api_call(url, method, args=None, headers=None, - http_headers=None, timeout=None, proxy=None, - verify=True, cert=None): + #: URL to proxy to. + self.proxy = None + + #: Verify HTTPS Certificate. + self.verify = True + + #: Client certificate path. + self.cert = None + + #: InitParameter/identifier of an object. + self.identifier = None + + #: SoftLayer mask (dict or string). + self.mask = None + + #: SoftLayer Filter (dict). + self.filter = None + + #: Integer result limit. + self.limit = None + + #: Integer result offset. + self.offset = None + + +def make_xml_rpc_api_call(request): """Makes a SoftLayer API call against the XML-RPC endpoint. - :param string uri: endpoint URL - :param string method: method to call E.G.: 'getObject' - :param dict headers: XML-RPC headers to use for the request - :param dict http_headers: HTTP headers to use for the request - :param int timeout: number of seconds to use as a timeout - :param bool verify: verify SSL cert - :param cert: client certificate path + :param request request: Request object """ - if args is None: - args = tuple() try: - largs = list(args) - largs.insert(0, {'headers': headers}) + largs = list(request.args) + + headers = request.headers + + if request.identifier is not None: + header_name = request.service + 'InitParameters' + headers[header_name] = {'id': request.identifier} + if request.mask is not None: + headers.update(__format_object_mask(request.mask, request.service)) + + if request.filter is not None: + headers['%sObjectFilter' % request.service] = request.filter + + if request.limit: + headers['resultLimit'] = { + 'limit': request.limit, + 'offset': request.offset or 0, + } + + largs.insert(0, {'headers': headers or None}) + + url = '/'.join([request.endpoint, request.service]) payload = utils.xmlrpc_client.dumps(tuple(largs), - methodname=method, + methodname=request.method, allow_none=True) LOGGER.debug("=== REQUEST ===") LOGGER.info('POST %s', url) - LOGGER.debug(http_headers) + LOGGER.debug(request.transport_headers) LOGGER.debug(payload) response = requests.request('POST', url, data=payload, - headers=http_headers, - timeout=timeout, - verify=verify, - cert=cert, - proxies=_proxies_dict(proxy)) + headers=request.transport_headers, + timeout=request.timeout, + verify=request.verify, + cert=request.cert, + proxies=_proxies_dict(request.proxy)) LOGGER.debug("=== RESPONSE ===") LOGGER.debug(response.headers) LOGGER.debug(response.content) @@ -86,38 +141,71 @@ def make_xml_rpc_api_call(url, method, args=None, headers=None, raise exceptions.TransportError(0, str(ex)) -def make_rest_api_call(method, url, - http_headers=None, timeout=None, proxy=None): +def make_rest_api_call(request, extension='json'): """Makes a SoftLayer API call against the REST endpoint. - :param string method: HTTP method: GET, POST, PUT, DELETE - :param string url: endpoint URL - :param dict http_headers: HTTP headers to use for the request - :param int timeout: number of seconds to use as a timeout + This currently only works with GET requests + + :param request request: Request object """ + url_parts = [request.endpoint, + request.service, + request.method] + if request.identifier is not None: + url_parts.append(str(request.identifier)) + + url = '%s.%s' % ('/'.join(url_parts), extension) + LOGGER.debug("=== REQUEST ===") - LOGGER.info('%s %s', method, url) - LOGGER.debug(http_headers) + LOGGER.info('%s %s', request.method, url) + LOGGER.debug(request.transport_headers) try: - resp = requests.request(method, url, - headers=http_headers, - timeout=timeout, - proxies=_proxies_dict(proxy)) + resp = requests.request('GET', url, + headers=request.transport_headers, + timeout=request.timeout, + proxies=_proxies_dict(request.proxy)) LOGGER.debug("=== RESPONSE ===") LOGGER.debug(resp.headers) LOGGER.debug(resp.content) resp.raise_for_status() - if url.endswith('.json'): + if extension == 'json': return json.loads(resp.content) else: return resp.text except requests.HTTPError as ex: - if url.endswith('.json'): + if extension == 'json': content = json.loads(ex.response.content) raise exceptions.SoftLayerAPIError(ex.response.status_code, content['error']) else: raise exceptions.SoftLayerAPIError(ex.response.status_code, - ex.response.text) + ex.response.content) except requests.RequestException as ex: raise exceptions.TransportError(0, str(ex)) + + +def _proxies_dict(proxy): + """Makes a dict appropriate to pass to requests.""" + if not proxy: + return None + return {'http': proxy, 'https': proxy} + + +def __format_object_mask(objectmask, service): + """Format new and old style object masks into proper headers. + + :param objectmask: a string- or dict-based object mask + :param service: a SoftLayer API service name + + """ + if isinstance(objectmask, dict): + mheader = '%sObjectMask' % service + else: + mheader = 'SoftLayer_ObjectMask' + + objectmask = objectmask.strip() + if (not objectmask.startswith('mask') + and not objectmask.startswith('[')): + objectmask = "mask[%s]" % objectmask + + return {mheader: {'mask': objectmask}} diff --git a/setup.cfg b/setup.cfg index d53bef49e..3d9f45499 100644 --- a/setup.cfg +++ b/setup.cfg @@ -2,7 +2,7 @@ verbosity=2 detailed-errors=1 with-coverage=1 -cover-min-percentage=74 +cover-min-percentage=75 cover-erase=true cover-package=SoftLayer cover-html=1 From e3045fa10bc71fa29ca9fc3f889feeed06b45b32 Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Mon, 22 Sep 2014 17:11:33 -0500 Subject: [PATCH 2/7] Removes ability to specify which rest file extension to use --- SoftLayer/managers/metadata.py | 6 +--- SoftLayer/tests/managers/metadata_tests.py | 2 -- SoftLayer/tests/transport_tests.py | 34 ++-------------------- SoftLayer/transports.py | 19 ++++-------- 4 files changed, 9 insertions(+), 52 deletions(-) diff --git a/SoftLayer/managers/metadata.py b/SoftLayer/managers/metadata.py index 6d6b47b2a..074415997 100644 --- a/SoftLayer/managers/metadata.py +++ b/SoftLayer/managers/metadata.py @@ -69,9 +69,6 @@ def get(self, name, param=None): raise exceptions.SoftLayerError('Unknown metadata attribute.') call_details = self.attribs[name] - extension = 'json' - if self.attribs[name]['call'] == 'UserMetadata': - extension = 'txt' if call_details.get('param_req'): if not param: @@ -87,8 +84,7 @@ def get(self, name, param=None): request.identifier = param try: - return transports.make_rest_api_call(request, - extension=extension) + return transports.make_rest_api_call(request) except exceptions.SoftLayerAPIError as ex: if ex.faultCode == 404: return None diff --git a/SoftLayer/tests/managers/metadata_tests.py b/SoftLayer/tests/managers/metadata_tests.py index 93681c052..592924e15 100644 --- a/SoftLayer/tests/managers/metadata_tests.py +++ b/SoftLayer/tests/managers/metadata_tests.py @@ -32,7 +32,6 @@ def test_get(self, make_request): {'User-Agent': consts.USER_AGENT}) self.assertEqual(request.timeout, self.metadata.timeout) self.assertEqual(request.identifier, None) - self.assertEqual(kwargs['extension'], 'json') @mock.patch('SoftLayer.transports.make_rest_api_call') def test_no_param(self, make_request): @@ -64,7 +63,6 @@ def test_user_data(self, make_request): (request, ), kwargs = make_request.call_args self.assertEqual(request.method, 'UserMetadata') self.assertEqual(request.identifier, None) - self.assertEqual(kwargs['extension'], 'txt') @mock.patch('SoftLayer.transports.make_rest_api_call') def test_return_none(self, make_request): diff --git a/SoftLayer/tests/transport_tests.py b/SoftLayer/tests/transport_tests.py index 8d02b0236..972653a64 100644 --- a/SoftLayer/tests/transport_tests.py +++ b/SoftLayer/tests/transport_tests.py @@ -247,7 +247,7 @@ def test_json(self, request): req.service = 'SoftLayer_Service' req.method = 'Resource' - resp = transports.make_rest_api_call(req, extension='json') + resp = transports.make_rest_api_call(req) self.assertEqual(resp, {}) request.assert_called_with( 'GET', 'http://something.com/SoftLayer_Service/Resource.json', @@ -266,8 +266,7 @@ def test_json(self, request): request().raise_for_status.side_effect = e self.assertRaises( - SoftLayer.SoftLayerAPIError, - transports.make_rest_api_call, req, extension='json') + SoftLayer.SoftLayerAPIError, transports.make_rest_api_call, req) def test_proxy_without_protocol(self): req = transports.Request() @@ -317,35 +316,6 @@ def test_with_id(self, request): proxies=None, timeout=None) - @mock.patch('requests.request') - def test_text(self, request): - request().content = 'content' - request().text = 'content' - - req = transports.Request() - req.endpoint = 'http://something.com' - req.service = 'SoftLayer_Service' - req.method = 'Resource' - - resp = transports.make_rest_api_call(req, extension='txt') - self.assertEqual(resp, 'content') - request.assert_called_with( - 'GET', - 'http://something.com/SoftLayer_Service/Resource.txt', - headers=None, - proxies=None, - timeout=None) - - # Test Text Error - e = requests.HTTPError('error') - e.response = mock.MagicMock() - e.response.status_code = 404 - e.response.content = 'Error Code' - request().raise_for_status.side_effect = e - - self.assertRaises(SoftLayer.SoftLayerAPIError, - transports.make_rest_api_call, req, extension='txt') - @mock.patch('requests.request') def test_unknown_error(self, request): e = requests.RequestException('error') diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 76a0ea6f2..4b23ddc4b 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -141,7 +141,7 @@ def make_xml_rpc_api_call(request): raise exceptions.TransportError(0, str(ex)) -def make_rest_api_call(request, extension='json'): +def make_rest_api_call(request): """Makes a SoftLayer API call against the REST endpoint. This currently only works with GET requests @@ -154,7 +154,7 @@ def make_rest_api_call(request, extension='json'): if request.identifier is not None: url_parts.append(str(request.identifier)) - url = '%s.%s' % ('/'.join(url_parts), extension) + url = '%s.%s' % ('/'.join(url_parts), 'json') LOGGER.debug("=== REQUEST ===") LOGGER.info('%s %s', request.method, url) @@ -168,18 +168,11 @@ def make_rest_api_call(request, extension='json'): LOGGER.debug(resp.headers) LOGGER.debug(resp.content) resp.raise_for_status() - if extension == 'json': - return json.loads(resp.content) - else: - return resp.text + return json.loads(resp.content) except requests.HTTPError as ex: - if extension == 'json': - content = json.loads(ex.response.content) - raise exceptions.SoftLayerAPIError(ex.response.status_code, - content['error']) - else: - raise exceptions.SoftLayerAPIError(ex.response.status_code, - ex.response.content) + content = json.loads(ex.response.content) + raise exceptions.SoftLayerAPIError(ex.response.status_code, + content['error']) except requests.RequestException as ex: raise exceptions.TransportError(0, str(ex)) From 36aa8eb3e04256b7716cc57b069bf72d87803426 Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Mon, 22 Sep 2014 17:30:04 -0500 Subject: [PATCH 3/7] Fixes user-metadata bug/clearifies debugging output --- SoftLayer/CLI/modules/metadata.py | 2 +- SoftLayer/transports.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/SoftLayer/CLI/modules/metadata.py b/SoftLayer/CLI/modules/metadata.py index 07f6c12f3..4bf89d9b4 100644 --- a/SoftLayer/CLI/modules/metadata.py +++ b/SoftLayer/CLI/modules/metadata.py @@ -181,7 +181,7 @@ def _execute(self, _): separator=',') -class UserMetadata(environment.CLIRunnable): +class UserMetadata(MetaRunnable): """ usage: sl metadata user_data [options] diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 4b23ddc4b..05c02f3b2 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -157,7 +157,7 @@ def make_rest_api_call(request): url = '%s.%s' % ('/'.join(url_parts), 'json') LOGGER.debug("=== REQUEST ===") - LOGGER.info('%s %s', request.method, url) + LOGGER.info(url) LOGGER.debug(request.transport_headers) try: resp = requests.request('GET', url, From c987c8309628caaa517fd74f15483f8863d3cd75 Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Mon, 22 Sep 2014 17:50:52 -0500 Subject: [PATCH 4/7] Don't set headers to None when they're empty --- SoftLayer/tests/transport_tests.py | 4 +++- SoftLayer/transports.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/SoftLayer/tests/transport_tests.py b/SoftLayer/tests/transport_tests.py index 972653a64..4afec4f1c 100644 --- a/SoftLayer/tests/transport_tests.py +++ b/SoftLayer/tests/transport_tests.py @@ -39,7 +39,9 @@ def test_call(self, request): headers - + + + diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 05c02f3b2..2e9ff64c5 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -94,7 +94,7 @@ def make_xml_rpc_api_call(request): 'offset': request.offset or 0, } - largs.insert(0, {'headers': headers or None}) + largs.insert(0, {'headers': headers}) url = '/'.join([request.endpoint, request.service]) payload = utils.xmlrpc_client.dumps(tuple(largs), From 0d7e32f627bb65fbd5c31e1ba55ce3e1019ab035 Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Mon, 22 Sep 2014 18:07:29 -0500 Subject: [PATCH 5/7] Proxies missing a protocol raises differently in py2.6 + misc changes --- SoftLayer/tests/transport_tests.py | 18 ++++++++++++++---- SoftLayer/transports.py | 4 ++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/SoftLayer/tests/transport_tests.py b/SoftLayer/tests/transport_tests.py index 4afec4f1c..5f166002b 100644 --- a/SoftLayer/tests/transport_tests.py +++ b/SoftLayer/tests/transport_tests.py @@ -4,6 +4,8 @@ :license: MIT, see LICENSE for more details. """ +import warnings + import mock import requests @@ -71,8 +73,12 @@ def test_proxy_without_protocol(self): req.method = 'Resource' req.proxy = 'localhost:3128' - self.assertRaises(SoftLayer.TransportError, - transports.make_xml_rpc_api_call, req) + try: + self.assertRaises(SoftLayer.TransportError, + transports.make_xml_rpc_api_call, req) + except AssertionError: + warnings.warn("AssertionError raised instead of a " + "SoftLayer.TransportError error") @mock.patch('requests.request') def test_valid_proxy(self, request): @@ -277,8 +283,12 @@ def test_proxy_without_protocol(self): req.method = 'Resource' req.proxy = 'localhost:3128' - self.assertRaises(SoftLayer.TransportError, - transports.make_rest_api_call, req) + try: + self.assertRaises(SoftLayer.TransportError, + transports.make_rest_api_call, req) + except AssertionError: + warnings.warn("AssertionError raised instead of a " + "SoftLayer.TransportError error") @mock.patch('requests.request') def test_valid_proxy(self, request): diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 2e9ff64c5..389e8b443 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -32,7 +32,7 @@ def __init__(self): self.method = None #: RPC Arguments. - self.args = [] + self.args = tuple() #: Transport headers. self.headers = {} @@ -43,7 +43,7 @@ def __init__(self): #: Integer timeout. self.timeout = None - #: URL to proxy to. + #: URL to proxy API requests to. self.proxy = None #: Verify HTTPS Certificate. From 0317a5606ec456dafc95869c586fa01bcbca4ac6 Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Tue, 23 Sep 2014 10:28:20 -0500 Subject: [PATCH 6/7] Clarifies doc blocks --- SoftLayer/tests/transport_tests.py | 14 +++++++------- SoftLayer/transports.py | 22 +++++++++++----------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/SoftLayer/tests/transport_tests.py b/SoftLayer/tests/transport_tests.py index 5f166002b..c655e3b53 100644 --- a/SoftLayer/tests/transport_tests.py +++ b/SoftLayer/tests/transport_tests.py @@ -58,7 +58,7 @@ def test_call(self, request): request.assert_called_with('POST', 'http://something.com/SoftLayer_Service', - headers=None, + headers={}, proxies=None, data=data, timeout=None, @@ -94,7 +94,7 @@ def test_valid_proxy(self, request): request.assert_called_with( 'POST', mock.ANY, - headers=None, + headers={}, proxies={'https': 'http://localhost:3128', 'http': 'http://localhost:3128'}, data=mock.ANY, @@ -248,7 +248,7 @@ def test_request_exception(self, request): class TestRestAPICall(testing.TestCase): @mock.patch('requests.request') - def test_json(self, request): + def test_basic(self, request): request().content = '{}' req = transports.Request() req.endpoint = 'http://something.com' @@ -259,7 +259,7 @@ def test_json(self, request): self.assertEqual(resp, {}) request.assert_called_with( 'GET', 'http://something.com/SoftLayer_Service/Resource.json', - headers=None, + headers={}, proxies=None, timeout=None) @@ -273,8 +273,8 @@ def test_json(self, request): }''' request().raise_for_status.side_effect = e - self.assertRaises( - SoftLayer.SoftLayerAPIError, transports.make_rest_api_call, req) + self.assertRaises(SoftLayer.SoftLayerAPIError, + transports.make_rest_api_call, req) def test_proxy_without_protocol(self): req = transports.Request() @@ -324,7 +324,7 @@ def test_with_id(self, request): request.assert_called_with( 'GET', 'http://something.com/SoftLayer_Service/getObject/2.json', - headers=None, + headers={}, proxies=None, timeout=None) diff --git a/SoftLayer/transports.py b/SoftLayer/transports.py index 389e8b443..e199bea77 100644 --- a/SoftLayer/transports.py +++ b/SoftLayer/transports.py @@ -25,20 +25,20 @@ def __init__(self): #: The SoftLayer endpoint address. self.endpoint = None - #: Service name. + #: API service name. E.G. SoftLayer_Account self.service = None - #: Method name. + #: API method name. E.G. getObject self.method = None - #: RPC Arguments. + #: API Parameters. self.args = tuple() - #: Transport headers. + #: API headers, used for authentication, masks, limits, offsets, etc. self.headers = {} #: Transport headers. - self.transport_headers = None + self.transport_headers = {} #: Integer timeout. self.timeout = None @@ -46,10 +46,10 @@ def __init__(self): #: URL to proxy API requests to. self.proxy = None - #: Verify HTTPS Certificate. + #: Boolean specifying if the server certificate should be verified. self.verify = True - #: Client certificate path. + #: Client certificate file path. self.cert = None #: InitParameter/identifier of an object. @@ -111,7 +111,7 @@ def make_xml_rpc_api_call(request): timeout=request.timeout, verify=request.verify, cert=request.cert, - proxies=_proxies_dict(request.proxy)) + proxies=__proxies_dict(request.proxy)) LOGGER.debug("=== RESPONSE ===") LOGGER.debug(response.headers) LOGGER.debug(response.content) @@ -163,7 +163,7 @@ def make_rest_api_call(request): resp = requests.request('GET', url, headers=request.transport_headers, timeout=request.timeout, - proxies=_proxies_dict(request.proxy)) + proxies=__proxies_dict(request.proxy)) LOGGER.debug("=== RESPONSE ===") LOGGER.debug(resp.headers) LOGGER.debug(resp.content) @@ -177,8 +177,8 @@ def make_rest_api_call(request): raise exceptions.TransportError(0, str(ex)) -def _proxies_dict(proxy): - """Makes a dict appropriate to pass to requests.""" +def __proxies_dict(proxy): + """Makes a proxy dict appropriate to pass to requests.""" if not proxy: return None return {'http': proxy, 'https': proxy} From 99e5c21d202528e93748a2c789baad5b077f9ffa Mon Sep 17 00:00:00 2001 From: Kevin McDonald Date: Tue, 23 Sep 2014 10:29:24 -0500 Subject: [PATCH 7/7] Fixes small style issue --- SoftLayer/API.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SoftLayer/API.py b/SoftLayer/API.py index cab5e6921..6797cf3b6 100644 --- a/SoftLayer/API.py +++ b/SoftLayer/API.py @@ -163,7 +163,7 @@ def call(self, service, method, *args, **kwargs): request.method = method request.args = args request.transport_headers = http_headers - request.timeout = self. timeout + request.timeout = self.timeout request.proxy = self.proxy request.identifier = kwargs.get('id') request.mask = kwargs.get('mask')