Merge "Improve logging and error handling in base code"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
570853a6a9
|
|
@ -61,22 +61,20 @@ class Connector(object):
|
|||
url = parse.urljoin(self._url, path)
|
||||
# TODO(lucasagomes): We should mask the data to remove sensitive
|
||||
# information
|
||||
LOG.debug('Issuing a HTTP %(method)s request at %(url)s with '
|
||||
'the headers "%(headers)s" and data "%(data)s"',
|
||||
LOG.debug('HTTP request: %(method)s %(url)s; '
|
||||
'headers: %(headers)s; body: %(data)s',
|
||||
{'method': method, 'url': url, 'headers': headers,
|
||||
'data': data or ''})
|
||||
'data': data})
|
||||
try:
|
||||
response = session.request(method, url, data=data)
|
||||
except requests.ConnectionError as e:
|
||||
raise exceptions.ConnectionError(url=url, error=e)
|
||||
|
||||
LOG.debug('Response: Status code: %d', response.status_code)
|
||||
try:
|
||||
response.raise_for_status()
|
||||
except requests.HTTPError as e:
|
||||
raise exceptions.HTTPError(
|
||||
method=method, url=url, error=e,
|
||||
status_code=e.response.status_code)
|
||||
exceptions.raise_for_response(method, url, response)
|
||||
LOG.debug('HTTP response for %(method)s %(url)s: '
|
||||
'status code: %(code)s',
|
||||
{'method': method, 'url': url,
|
||||
'code': response.status_code})
|
||||
|
||||
return response
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,11 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import logging
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class SushyError(Exception):
|
||||
"""Basic exception for errors raised by Sushy"""
|
||||
|
|
@ -49,15 +54,71 @@ class HTTPError(SushyError):
|
|||
"""Basic exception for HTTP errors"""
|
||||
|
||||
status_code = None
|
||||
message = ('Error issuing a %(method)s request at %(url)s. '
|
||||
'Error: %(error)s')
|
||||
"""HTTP status code."""
|
||||
|
||||
def __init__(self, status_code=None, **kwargs):
|
||||
body = None
|
||||
"""Error JSON body, if present."""
|
||||
|
||||
code = 'Base.1.0.GeneralError'
|
||||
"""Error code defined in the Redfish specification, if present."""
|
||||
|
||||
detail = None
|
||||
"""Error message defined in the Redfish specification, if present."""
|
||||
|
||||
message = ('HTTP %(method)s %(url)s returned code %(code)s. %(error)s')
|
||||
|
||||
def __init__(self, method, url, response):
|
||||
self.status_code = response.status_code
|
||||
try:
|
||||
body = response.json()
|
||||
except ValueError:
|
||||
LOG.warning('Error response from %(method)s %(url)s '
|
||||
'with status code %(code)s has no JSON body',
|
||||
{'method': method, 'url': url, 'code':
|
||||
self.status_code})
|
||||
error = 'unknown error'
|
||||
else:
|
||||
# TODO(dtantsur): parse @Message.ExtendedInfo
|
||||
self.body = body.get('error', {})
|
||||
self.code = self.body.get('code', 'Base.1.0.GeneralError')
|
||||
self.detail = self.body.get('message')
|
||||
error = '%s: %s' % (self.code, self.detail or 'unknown error')
|
||||
|
||||
kwargs = {'method': method, 'url': url, 'code': self.status_code,
|
||||
'error': error}
|
||||
LOG.debug('HTTP response for %(method)s %(url)s: '
|
||||
'status code: %(code)s, error: %(error)s', kwargs)
|
||||
super(HTTPError, self).__init__(**kwargs)
|
||||
if status_code is not None:
|
||||
self.status_code = status_code
|
||||
|
||||
|
||||
class BadRequestError(HTTPError):
|
||||
pass
|
||||
|
||||
|
||||
class ResourceNotFoundError(HTTPError):
|
||||
status_code = 404
|
||||
message = 'Resource %(resource)s not found'
|
||||
# Overwrite the complex generic message with a simpler one.
|
||||
message = 'Resource %(url)s not found'
|
||||
|
||||
|
||||
class ServerSideError(HTTPError):
|
||||
pass
|
||||
|
||||
|
||||
class AccessError(HTTPError):
|
||||
pass
|
||||
|
||||
|
||||
def raise_for_response(method, url, response):
|
||||
"""Raise a correct error class, if needed."""
|
||||
if response.status_code < 400:
|
||||
return
|
||||
elif response.status_code == 404:
|
||||
raise ResourceNotFoundError(method, url, response)
|
||||
elif response.status_code == 400:
|
||||
raise BadRequestError(method, url, response)
|
||||
elif response.status_code in (401, 403):
|
||||
raise AccessError(method, url, response)
|
||||
elif response.status_code >= 500:
|
||||
raise ServerSideError(method, url, response)
|
||||
else:
|
||||
raise HTTPError(method, url, response)
|
||||
|
|
|
|||
|
|
@ -14,13 +14,16 @@
|
|||
# under the License.
|
||||
|
||||
import abc
|
||||
import logging
|
||||
|
||||
import six
|
||||
|
||||
from sushy import exceptions
|
||||
from sushy import utils
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@six.add_metaclass(abc.ABCMeta)
|
||||
class ResourceBase(object):
|
||||
|
||||
|
|
@ -52,14 +55,10 @@ class ResourceBase(object):
|
|||
:raises: ConnectionError
|
||||
:raises: HTTPError
|
||||
"""
|
||||
try:
|
||||
resp = self._conn.get(path=self._path)
|
||||
except exceptions.HTTPError as e:
|
||||
if e.status_code == 404:
|
||||
raise exceptions.ResourceNotFoundError(resource=self._path)
|
||||
raise
|
||||
|
||||
self._json = resp.json()
|
||||
self._json = self._conn.get(path=self._path).json()
|
||||
LOG.debug('Received representation of %(type)s %(path)s: %(json)s',
|
||||
{'type': self.__class__.__name__,
|
||||
'path': self._path, 'json': self._json})
|
||||
self._parse_attributes()
|
||||
|
||||
@property
|
||||
|
|
@ -114,6 +113,9 @@ class ResourceCollectionBase(ResourceBase):
|
|||
self.name = self.json.get('Name')
|
||||
self.members_identities = (
|
||||
utils.get_members_identities(self.json.get('Members', [])))
|
||||
LOG.debug('Received %(count)d member(s) for %(type)s %(path)s',
|
||||
{'count': len(self.members_identities),
|
||||
'type': self.__class__.__name__, 'path': self._path})
|
||||
|
||||
def get_member(self, identity):
|
||||
"""Given the identity return a ``_resource_type`` object
|
||||
|
|
|
|||
|
|
@ -0,0 +1,35 @@
|
|||
{
|
||||
"error": {
|
||||
"code": "Base.1.0.GeneralError",
|
||||
"message": "A general error has occurred. See ExtendedInfo for more information.",
|
||||
"@Message.ExtendedInfo": [
|
||||
{
|
||||
"@odata.type" : "/redfish/v1/$metadata#Message.1.0.0.Message",
|
||||
"MessageId": "Base.1.0.PropertyValueNotInList",
|
||||
"RelatedProperties": [
|
||||
"#/IndicatorLED"
|
||||
],
|
||||
"Message": "The value Red for the property IndicatorLED is not in the list of acceptable values",
|
||||
"MessageArgs": [
|
||||
"RED",
|
||||
"IndicatorLED"
|
||||
],
|
||||
"Severity": "Warning",
|
||||
"Resolution": "Remove the property from the request body and resubmit the request if the operation failed"
|
||||
},
|
||||
{
|
||||
"@odata.type" : "/redfish/v1/$metadata#Message.1.0.0.Message",
|
||||
"MessageId": "Base.1.0.PropertyNotWriteable",
|
||||
"RelatedProperties": [
|
||||
"#/SKU"
|
||||
],
|
||||
"Message": "The property SKU is a read only property and cannot be assigned a value",
|
||||
"MessageArgs": [
|
||||
"SKU"
|
||||
],
|
||||
"Severity": "Warning",
|
||||
"Resolution": "Remove the property from the request body and resubmit the request if the operation failed"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
|
@ -20,7 +20,7 @@ from sushy.resources import base as resource_base
|
|||
from sushy.tests.unit import base
|
||||
|
||||
|
||||
class BaseResouce(resource_base.ResourceBase):
|
||||
class BaseResource(resource_base.ResourceBase):
|
||||
|
||||
def _parse_attributes(self):
|
||||
pass
|
||||
|
|
@ -31,8 +31,8 @@ class ResourceBaseTestCase(base.TestCase):
|
|||
def setUp(self):
|
||||
super(ResourceBaseTestCase, self).setUp()
|
||||
self.conn = mock.Mock()
|
||||
self.base_resource = BaseResouce(connector=self.conn, path='/Foo',
|
||||
redfish_version='1.0.2')
|
||||
self.base_resource = BaseResource(connector=self.conn, path='/Foo',
|
||||
redfish_version='1.0.2')
|
||||
# refresh() is called in the constructor
|
||||
self.conn.reset_mock()
|
||||
|
||||
|
|
@ -40,23 +40,8 @@ class ResourceBaseTestCase(base.TestCase):
|
|||
self.base_resource.refresh()
|
||||
self.conn.get.assert_called_once_with(path='/Foo')
|
||||
|
||||
def test_refresh_http_error_reraised(self):
|
||||
self.conn.get.side_effect = exceptions.HTTPError(
|
||||
method='GET', url='http://foo.bar:8000/redfish/v1', error='boom',
|
||||
status_code=404)
|
||||
self.assertRaises(exceptions.ResourceNotFoundError,
|
||||
self.base_resource.refresh)
|
||||
self.conn.get.assert_called_once_with(path='/Foo')
|
||||
|
||||
def test_refresh_resource_not_found(self):
|
||||
self.conn.get.side_effect = exceptions.HTTPError(
|
||||
method='GET', url='http://foo.bar:8000/redfish/v1', error='boom',
|
||||
status_code=400)
|
||||
self.assertRaises(exceptions.HTTPError, self.base_resource.refresh)
|
||||
self.conn.get.assert_called_once_with(path='/Foo')
|
||||
|
||||
|
||||
class TestResouce(resource_base.ResourceBase):
|
||||
class TestResource(resource_base.ResourceBase):
|
||||
"""A concrete Test Resource to test against"""
|
||||
|
||||
def __init__(self, connector, identity, redfish_version=None):
|
||||
|
|
@ -67,20 +52,20 @@ class TestResouce(resource_base.ResourceBase):
|
|||
:param redfish_version: The version of RedFish. Used to construct
|
||||
the object according to schema of the given version.
|
||||
"""
|
||||
super(TestResouce, self).__init__(connector, 'Fakes/%s' % identity,
|
||||
redfish_version)
|
||||
super(TestResource, self).__init__(connector, 'Fakes/%s' % identity,
|
||||
redfish_version)
|
||||
self.identity = identity
|
||||
|
||||
def _parse_attributes(self):
|
||||
pass
|
||||
|
||||
|
||||
class TestResouceCollection(resource_base.ResourceCollectionBase):
|
||||
class TestResourceCollection(resource_base.ResourceCollectionBase):
|
||||
"""A concrete Test Resource Collection to test against"""
|
||||
|
||||
@property
|
||||
def _resource_type(self):
|
||||
return TestResouce
|
||||
return TestResource
|
||||
|
||||
def __init__(self, connector, redfish_version=None):
|
||||
"""Ctor of TestResourceCollection
|
||||
|
|
@ -89,8 +74,8 @@ class TestResouceCollection(resource_base.ResourceCollectionBase):
|
|||
:param redfish_version: The version of RedFish. Used to construct
|
||||
the object according to schema of the given version.
|
||||
"""
|
||||
super(TestResouceCollection, self).__init__(connector, 'Fakes',
|
||||
redfish_version)
|
||||
super(TestResourceCollection, self).__init__(connector, 'Fakes',
|
||||
redfish_version)
|
||||
|
||||
|
||||
class ResourceCollectionBaseTestCase(base.TestCase):
|
||||
|
|
@ -98,7 +83,7 @@ class ResourceCollectionBaseTestCase(base.TestCase):
|
|||
def setUp(self):
|
||||
super(ResourceCollectionBaseTestCase, self).setUp()
|
||||
self.conn = mock.MagicMock()
|
||||
self.test_resource_collection = TestResouceCollection(
|
||||
self.test_resource_collection = TestResourceCollection(
|
||||
self.conn, redfish_version='1.0.x')
|
||||
self.conn.reset_mock()
|
||||
|
||||
|
|
@ -109,7 +94,7 @@ class ResourceCollectionBaseTestCase(base.TestCase):
|
|||
# | WHEN |
|
||||
result = self.test_resource_collection.get_member('1')
|
||||
# | THEN |
|
||||
self.assertTrue(isinstance(result, TestResouce))
|
||||
self.assertTrue(isinstance(result, TestResource))
|
||||
self.assertEqual('1', result.identity)
|
||||
self.assertEqual('1.0.x', result.redfish_version)
|
||||
|
||||
|
|
@ -117,9 +102,9 @@ class ResourceCollectionBaseTestCase(base.TestCase):
|
|||
# | GIVEN |
|
||||
# setting a valid member identity
|
||||
self.test_resource_collection.members_identities = ('1',)
|
||||
self.conn.get.side_effect = exceptions.HTTPError(
|
||||
self.conn.get.side_effect = exceptions.ResourceNotFoundError(
|
||||
method='GET', url='http://foo.bar:8000/redfish/v1/Fakes/2',
|
||||
error='boom', status_code=404)
|
||||
response=mock.Mock(status_code=404))
|
||||
# | WHEN & THEN |
|
||||
self.assertRaises(exceptions.ResourceNotFoundError,
|
||||
self.test_resource_collection.get_member, '2')
|
||||
|
|
@ -135,6 +120,6 @@ class ResourceCollectionBaseTestCase(base.TestCase):
|
|||
# | THEN |
|
||||
self.assertTrue(isinstance(result, list))
|
||||
for val in result:
|
||||
self.assertTrue(isinstance(val, TestResouce))
|
||||
self.assertTrue(isinstance(val, TestResource))
|
||||
self.assertTrue(val.identity in member_ids)
|
||||
self.assertEqual('1.0.x', val.redfish_version)
|
||||
|
|
|
|||
|
|
@ -13,6 +13,9 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import json
|
||||
|
||||
import fixtures
|
||||
import mock
|
||||
import requests
|
||||
|
||||
|
|
@ -21,10 +24,10 @@ from sushy import exceptions
|
|||
from sushy.tests.unit import base
|
||||
|
||||
|
||||
class ConnectorTestCase(base.TestCase):
|
||||
class ConnectorMethodsTestCase(base.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(ConnectorTestCase, self).setUp()
|
||||
super(ConnectorMethodsTestCase, self).setUp()
|
||||
self.conn = connector.Connector(
|
||||
'http://foo.bar:1234', username='user',
|
||||
password='pass', verify=True)
|
||||
|
|
@ -49,43 +52,98 @@ class ConnectorTestCase(base.TestCase):
|
|||
mock__op.assert_called_once_with(mock.ANY, 'PATCH', 'fake/path',
|
||||
self.data, self.headers)
|
||||
|
||||
@mock.patch('sushy.connector.requests.Session', autospec=True)
|
||||
def _test_op(self, headers, expected_headers, mock_session):
|
||||
fake_session = mock.Mock()
|
||||
mock_session.return_value.__enter__.return_value = fake_session
|
||||
|
||||
class ConnectorOpTestCase(base.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(ConnectorOpTestCase, self).setUp()
|
||||
self.conn = connector.Connector(
|
||||
'http://foo.bar:1234', username='user',
|
||||
password='pass', verify=True)
|
||||
self.data = {'fake': 'data'}
|
||||
self.headers = {'X-Fake': 'header'}
|
||||
self.session_fixture = self.useFixture(
|
||||
fixtures.MockPatchObject(connector.requests, 'Session',
|
||||
autospec=True)
|
||||
)
|
||||
self.session = (
|
||||
self.session_fixture.mock.return_value.__enter__.return_value)
|
||||
self.request = self.session.request
|
||||
|
||||
def _test_op(self, headers, expected_headers):
|
||||
self.request.return_value.status_code = 200
|
||||
|
||||
self.conn._op('GET', path='fake/path', data=self.data,
|
||||
headers=headers)
|
||||
mock_session.assert_called_once_with()
|
||||
fake_session.request.assert_called_once_with(
|
||||
self.request.assert_called_once_with(
|
||||
'GET', 'http://foo.bar:1234/fake/path',
|
||||
data='{"fake": "data"}')
|
||||
self.assertEqual(expected_headers, fake_session.headers)
|
||||
self.assertEqual(expected_headers, self.session.headers)
|
||||
|
||||
def test__op(self):
|
||||
def test_ok_with_headers(self):
|
||||
expected_headers = self.headers.copy()
|
||||
expected_headers['Content-Type'] = 'application/json'
|
||||
self._test_op(self.headers, expected_headers)
|
||||
|
||||
def test__op_no_headers(self):
|
||||
def test_ok_no_headers(self):
|
||||
expected_headers = {'Content-Type': 'application/json'}
|
||||
self._test_op(None, expected_headers)
|
||||
|
||||
@mock.patch('sushy.connector.requests.Session', autospec=True)
|
||||
def test__op_connection_error(self, mock_session):
|
||||
fake_session = mock.Mock()
|
||||
mock_session.return_value.__enter__.return_value = fake_session
|
||||
fake_session.request.side_effect = requests.exceptions.ConnectionError
|
||||
|
||||
def test_connection_error(self):
|
||||
self.request.side_effect = requests.exceptions.ConnectionError
|
||||
self.assertRaises(exceptions.ConnectionError, self.conn._op, 'GET')
|
||||
|
||||
@mock.patch('sushy.connector.requests.Session', autospec=True)
|
||||
def test__op_http_error(self, mock_session):
|
||||
fake_session = mock.Mock()
|
||||
mock_session.return_value.__enter__.return_value = fake_session
|
||||
fake_response = fake_session.request.return_value
|
||||
fake_response.status_code = 400
|
||||
fake_response.raise_for_status.side_effect = (
|
||||
requests.exceptions.HTTPError(response=fake_response))
|
||||
def test_unknown_http_error(self):
|
||||
self.request.return_value.status_code = 409
|
||||
self.request.return_value.json.side_effect = ValueError('no json')
|
||||
|
||||
self.assertRaises(exceptions.HTTPError, self.conn._op, 'GET')
|
||||
with self.assertRaisesRegex(exceptions.HTTPError,
|
||||
'unknown error') as cm:
|
||||
self.conn._op('GET', 'http://foo.bar')
|
||||
exc = cm.exception
|
||||
self.assertEqual(409, exc.status_code)
|
||||
self.assertIsNone(exc.body)
|
||||
self.assertIsNone(exc.detail)
|
||||
|
||||
def test_known_http_error(self):
|
||||
self.request.return_value.status_code = 400
|
||||
with open('sushy/tests/unit/json_samples/error.json', 'r') as f:
|
||||
self.request.return_value.json.return_value = json.load(f)
|
||||
|
||||
with self.assertRaisesRegex(exceptions.BadRequestError,
|
||||
'A general error has occurred') as cm:
|
||||
self.conn._op('GET', 'http://foo.bar')
|
||||
exc = cm.exception
|
||||
self.assertEqual(400, exc.status_code)
|
||||
self.assertIsNotNone(exc.body)
|
||||
self.assertIn('A general error has occurred', exc.detail)
|
||||
|
||||
def test_not_found_error(self):
|
||||
self.request.return_value.status_code = 404
|
||||
self.request.return_value.json.side_effect = ValueError('no json')
|
||||
|
||||
with self.assertRaisesRegex(exceptions.ResourceNotFoundError,
|
||||
'Resource http://foo.bar not found') as cm:
|
||||
self.conn._op('GET', 'http://foo.bar')
|
||||
exc = cm.exception
|
||||
self.assertEqual(404, exc.status_code)
|
||||
|
||||
def test_server_error(self):
|
||||
self.request.return_value.status_code = 500
|
||||
self.request.return_value.json.side_effect = ValueError('no json')
|
||||
|
||||
with self.assertRaisesRegex(exceptions.ServerSideError,
|
||||
'unknown error') as cm:
|
||||
self.conn._op('GET', 'http://foo.bar')
|
||||
exc = cm.exception
|
||||
self.assertEqual(500, exc.status_code)
|
||||
|
||||
def test_access_error(self):
|
||||
self.request.return_value.status_code = 403
|
||||
self.request.return_value.json.side_effect = ValueError('no json')
|
||||
|
||||
with self.assertRaisesRegex(exceptions.AccessError,
|
||||
'unknown error') as cm:
|
||||
self.conn._op('GET', 'http://foo.bar')
|
||||
exc = cm.exception
|
||||
self.assertEqual(403, exc.status_code)
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ oslotest>=1.10.0 # Apache-2.0
|
|||
testrepository>=0.0.18 # Apache-2.0/BSD
|
||||
testscenarios>=0.4 # Apache-2.0/BSD
|
||||
testtools>=1.4.0 # MIT
|
||||
fixtures>=3.0.0 # Apache-2.0/BSD
|
||||
|
||||
# releasenotes
|
||||
reno>=1.8.0 # Apache-2.0
|
||||
|
|
|
|||
Loading…
Reference in New Issue