From e0ad907a361a71ce22e89e28bd319b8c8dc18432 Mon Sep 17 00:00:00 2001 From: Everett Toews Date: Thu, 7 Jan 2016 22:52:07 -0600 Subject: [PATCH] Map KSA exception to SDK exceptions The use of KSA is an implementation detail. Users should not have to know about KSA. Catch all KSA exceptions and map them to SDK exceptions. Change-Id: Ib873c038358fb318ae46e8bc5bd3b4dfb683daaa Closes-Bug: #1526516 --- openstack/connection.py | 18 +--------- openstack/exceptions.py | 24 +++++++++----- openstack/proxy.py | 19 +++++++---- openstack/resource.py | 5 ++- openstack/session.py | 37 +++++++++++++++++++-- openstack/tests/unit/test_connection.py | 37 --------------------- openstack/tests/unit/test_exceptions.py | 8 ++--- openstack/tests/unit/test_proxy.py | 16 ++++----- openstack/tests/unit/test_resource.py | 13 ++++---- openstack/tests/unit/test_session.py | 44 +++++++++++++++++++++++++ 10 files changed, 126 insertions(+), 95 deletions(-) diff --git a/openstack/connection.py b/openstack/connection.py index 40ff09a39..3f6515480 100644 --- a/openstack/connection.py +++ b/openstack/connection.py @@ -60,12 +60,9 @@ try to find it and if that fails, you would create it:: import logging import sys -from keystoneauth1 import exceptions as ksa_exc from keystoneauth1.loading import base as ksa_loader import os_client_config -import six -from openstack import exceptions from openstack import profile as _profile from openstack import proxy from openstack import session as _session @@ -250,19 +247,6 @@ class Connection(object): to be authorized or the `auth_plugin` argument is missing, etc. """ - try: - headers = self.session.get_auth_headers() - except ksa_exc.AuthorizationFailure as ex: - raise exceptions.HttpException("Authentication Failure", - six.text_type(ex), - status_code=401) - except ksa_exc.MissingAuthPlugin as ex: - raise exceptions.HttpException("Bad Request", - six.text_type(ex), - status_code=400) - except Exception as ex: - raise exceptions.HttpException("Unknown exception", - six.text_type(ex), - status_code=500) + headers = self.session.get_auth_headers() return headers.get('X-Auth-Token') if headers else None diff --git a/openstack/exceptions.py b/openstack/exceptions.py index d2d7e3786..f0048e03d 100644 --- a/openstack/exceptions.py +++ b/openstack/exceptions.py @@ -21,9 +21,10 @@ import six class SDKException(Exception): """The base exception class for all exceptions this library raises.""" - def __init__(self, message=None): + def __init__(self, message=None, cause=None): self.message = self.__class__.__name__ if message is None else message - super(Exception, self).__init__(self.message) + self.cause = cause + super(SDKException, self).__init__(self.message) class InvalidResponse(SDKException): @@ -35,10 +36,17 @@ class InvalidResponse(SDKException): class HttpException(SDKException): - def __init__(self, message, details=None, status_code=None): - super(HttpException, self).__init__(message) + + def __init__(self, message=None, details=None, response=None, + request_id=None, url=None, method=None, + http_status=None, cause=None): + super(HttpException, self).__init__(message=message, cause=cause) self.details = details - self.status_code = status_code + self.response = response + self.request_id = request_id + self.url = url + self.method = method + self.http_status = http_status def __unicode__(self): msg = self.__class__.__name__ + ": " + self.message @@ -58,9 +66,9 @@ class NotFoundException(HttpException): class MethodNotSupported(SDKException): """The resource does not support this operation type.""" def __init__(self, cls, method): - msg = ('The %s method is not supported for %s.%s' % - (method, cls.__module__, cls.__name__)) - super(Exception, self).__init__(msg) + message = ('The %s method is not supported for %s.%s' % + (method, cls.__module__, cls.__name__)) + super(MethodNotSupported, self).__init__(message=message) class DuplicateResource(SDKException): diff --git a/openstack/proxy.py b/openstack/proxy.py index e56d30202..a3f71c7a1 100644 --- a/openstack/proxy.py +++ b/openstack/proxy.py @@ -10,7 +10,6 @@ # License for the specific language governing permissions and limitations # under the License. -from keystoneauth1 import exceptions as ksa_exc from openstack import exceptions from openstack import resource @@ -124,14 +123,17 @@ class BaseProxy(object): try: rv = res.delete(self.session) - except ksa_exc.NotFound as exc: + except exceptions.NotFoundException as e: if ignore_missing: return None else: # Reraise with a more specific type and message raise exceptions.ResourceNotFound( - "No %s found for %s" % (resource_type.__name__, value), - details=exc.details, status_code=exc.http_status) + message="No %s found for %s" % + (resource_type.__name__, value), + details=e.details, response=e.response, + request_id=e.request_id, url=e.url, method=e.method, + http_status=e.http_status, cause=e.cause) return rv @@ -197,10 +199,13 @@ class BaseProxy(object): try: return res.get(self.session, args=args) - except ksa_exc.NotFound as exc: + except exceptions.NotFoundException as e: raise exceptions.ResourceNotFound( - "No %s found for %s" % (resource_type.__name__, value), - details=exc.details, status_code=exc.http_status) + message="No %s found for %s" % + (resource_type.__name__, value), + details=e.details, response=e.response, + request_id=e.request_id, url=e.url, method=e.method, + http_status=e.http_status, cause=e.cause) def _list(self, resource_type, value=None, paginated=False, path_args=None, **query): diff --git a/openstack/resource.py b/openstack/resource.py index ce8142c8a..f57fe0bec 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -35,7 +35,6 @@ import copy import itertools import time -from keystoneauth1 import exceptions as ksa_exc import six from six.moves.urllib import parse as url_parse @@ -947,7 +946,7 @@ class Resource(collections.MutableMapping): try: if cls.allow_retrieve: return cls.get_by_id(session, name_or_id, path_args=path_args) - except ksa_exc.NotFound: + except exceptions.NotFoundException: pass data = cls.list(session, path_args=path_args) @@ -1023,7 +1022,7 @@ def wait_for_delete(session, resource, interval, wait): while total_sleep < wait: try: resource.get(session) - except ksa_exc.NotFound: + except exceptions.NotFoundException: return resource time.sleep(interval) total_sleep += interval diff --git a/openstack/session.py b/openstack/session.py index 5eae6e4a4..5786748a1 100644 --- a/openstack/session.py +++ b/openstack/session.py @@ -12,15 +12,19 @@ """ The :class:`~openstack.session.Session` overrides -:class:`~keystoneauth1.session.Session` to provide end point filtering. +:class:`~keystoneauth1.session.Session` to provide end point filtering and +mapping KSA exceptions to SDK exceptions. """ import re -from six.moves.urllib import parse +from keystoneauth1 import exceptions as _exceptions from keystoneauth1 import session as _session import openstack +from openstack import exceptions + +from six.moves.urllib import parse DEFAULT_USER_AGENT = "openstacksdk/%s" % openstack.__version__ VERSION_PATTERN = re.compile('/v\d[\d.]*') @@ -40,6 +44,29 @@ def parse_url(filt, url): return url +def map_exceptions(func): + def map_exceptions_wrapper(*args, **kwargs): + try: + return func(*args, **kwargs) + except _exceptions.HttpError as e: + if e.http_status == 404: + raise exceptions.NotFoundException( + message=e.message, details=e.details, + response=e.response, request_id=e.request_id, + url=e.url, method=e.method, + http_status=e.http_status, cause=e) + else: + raise exceptions.HttpException( + message=e.message, details=e.details, + response=e.response, request_id=e.request_id, + url=e.url, method=e.method, + http_status=e.http_status, cause=e) + except _exceptions.ClientException as e: + raise exceptions.SDKException(message=e.message, cause=e) + + return map_exceptions_wrapper + + class Session(_session.Session): def __init__(self, profile, user_agent=None, **kwargs): @@ -66,7 +93,7 @@ class Session(_session.Session): self.profile = profile def get_endpoint(self, auth=None, interface=None, **kwargs): - """Override get endpoint to automate endpoint filering""" + """Override get endpoint to automate endpoint filtering""" service_type = kwargs.get('service_type') filt = self.profile.get_filter(service_type) @@ -74,3 +101,7 @@ class Session(_session.Session): filt.interface = interface url = super(Session, self).get_endpoint(auth, **filt.get_filter()) return parse_url(filt, url) + + @map_exceptions + def request(self, *args, **kwargs): + return super(Session, self).request(*args, **kwargs) diff --git a/openstack/tests/unit/test_connection.py b/openstack/tests/unit/test_connection.py index 670b1135b..013794329 100644 --- a/openstack/tests/unit/test_connection.py +++ b/openstack/tests/unit/test_connection.py @@ -13,13 +13,10 @@ import os import tempfile -from keystoneauth1 import exceptions as ksa_exc import mock import os_client_config -import six from openstack import connection -from openstack import exceptions from openstack import profile from openstack.tests.unit import base @@ -193,37 +190,3 @@ class TestConnection(base.TestCase): authenticator=mock.Mock()) res = sot.authorize() self.assertIsNone(res) - - def test_authorize_not_authorized(self): - fake_session = mock.Mock() - ex_auth = ksa_exc.AuthorizationFailure("not authorized") - fake_session.get_auth_headers.side_effect = ex_auth - - sot = connection.Connection(session=fake_session, - authenticator=mock.Mock()) - ex = self.assertRaises(exceptions.HttpException, sot.authorize) - self.assertEqual(401, ex.status_code) - self.assertEqual('HttpException: Authentication Failure, not ' - 'authorized', six.text_type(ex)) - - def test_authorize_no_authplugin(self): - fake_session = mock.Mock() - ex_auth = ksa_exc.MissingAuthPlugin("missing auth plugin") - fake_session.get_auth_headers.side_effect = ex_auth - sot = connection.Connection(session=fake_session, - authenticator=mock.Mock()) - ex = self.assertRaises(exceptions.HttpException, sot.authorize) - self.assertEqual(400, ex.status_code) - self.assertEqual('HttpException: Bad Request, missing auth plugin', - six.text_type(ex)) - - def test_authorize_other_exceptions(self): - fake_session = mock.Mock() - fake_session.get_auth_headers.side_effect = Exception() - - sot = connection.Connection(session=fake_session, - authenticator=mock.Mock()) - ex = self.assertRaises(exceptions.HttpException, sot.authorize) - self.assertEqual(500, ex.status_code) - self.assertEqual('HttpException: Unknown exception', - six.text_type(ex)) diff --git a/openstack/tests/unit/test_exceptions.py b/openstack/tests/unit/test_exceptions.py index 9ad92cefd..54a95b1fb 100644 --- a/openstack/tests/unit/test_exceptions.py +++ b/openstack/tests/unit/test_exceptions.py @@ -47,11 +47,11 @@ class Test_HttpException(testtools.TestCase): self.assertEqual(self.message, exc.message) self.assertEqual(details, exc.details) - def test_status_code(self): - status_code = 123 + def test_http_status(self): + http_status = 123 exc = self.assertRaises(exceptions.HttpException, self._do_raise, self.message, - status_code=status_code) + http_status=http_status) self.assertEqual(self.message, exc.message) - self.assertEqual(status_code, exc.status_code) + self.assertEqual(http_status, exc.http_status) diff --git a/openstack/tests/unit/test_proxy.py b/openstack/tests/unit/test_proxy.py index a016c6dec..311bd6439 100644 --- a/openstack/tests/unit/test_proxy.py +++ b/openstack/tests/unit/test_proxy.py @@ -13,8 +13,6 @@ import mock import testtools -from keystoneauth1 import exceptions as ksa_exc - from openstack import exceptions from openstack import proxy from openstack import resource @@ -119,15 +117,15 @@ class TestProxyDelete(testtools.TestCase): self.assertEqual(rv, self.fake_id) def test_delete_ignore_missing(self): - self.res.delete.side_effect = ksa_exc.NotFound(message="test", - http_status=404) + self.res.delete.side_effect = exceptions.NotFoundException( + message="test", http_status=404) rv = self.sot._delete(DeleteableResource, self.fake_id) self.assertIsNone(rv) def test_delete_ResourceNotFound(self): - self.res.delete.side_effect = ksa_exc.NotFound(message="test", - http_status=404) + self.res.delete.side_effect = exceptions.NotFoundException( + message="test", http_status=404) self.assertRaisesRegexp( exceptions.ResourceNotFound, @@ -137,7 +135,7 @@ class TestProxyDelete(testtools.TestCase): def test_delete_HttpException(self): self.res.delete.side_effect = exceptions.HttpException( - message="test", status_code=500) + message="test", http_status=500) self.assertRaises(exceptions.HttpException, self.sot._delete, DeleteableResource, self.res, ignore_missing=False) @@ -238,8 +236,8 @@ class TestProxyGet(testtools.TestCase): self.assertEqual(rv, self.fake_result) def test_get_not_found(self): - self.res.get.side_effect = ksa_exc.NotFound(message="test", - http_status=404) + self.res.get.side_effect = exceptions.NotFoundException( + message="test", http_status=404) self.assertRaisesRegexp( exceptions.ResourceNotFound, diff --git a/openstack/tests/unit/test_resource.py b/openstack/tests/unit/test_resource.py index 7203d8359..4934ac975 100644 --- a/openstack/tests/unit/test_resource.py +++ b/openstack/tests/unit/test_resource.py @@ -14,7 +14,6 @@ import copy import json import os -from keystoneauth1 import exceptions as ksa_exceptions from keystoneauth1 import session import mock import requests @@ -1287,7 +1286,7 @@ class TestFind(base.TestCase): def test_name(self): self.mock_get.side_effect = [ - ksa_exceptions.http.NotFound(), + exceptions.NotFoundException(), FakeResponse({FakeResource.resources_key: [self.matrix]}) ] @@ -1330,7 +1329,7 @@ class TestFind(base.TestCase): dupe['id'] = 'different' self.mock_get.side_effect = [ # Raise a 404 first so we get out of the ID search and into name. - ksa_exceptions.http.NotFound(), + exceptions.NotFoundException(), FakeResponse({FakeResource.resources_key: [self.matrix, dupe]}) ] @@ -1358,7 +1357,7 @@ class TestFind(base.TestCase): def test_nada(self): self.mock_get.side_effect = [ - ksa_exceptions.http.NotFound(), + exceptions.NotFoundException(), FakeResponse({FakeResource.resources_key: []}) ] @@ -1366,7 +1365,7 @@ class TestFind(base.TestCase): def test_no_name(self): self.mock_get.side_effect = [ - ksa_exceptions.http.NotFound(), + exceptions.NotFoundException(), FakeResponse({FakeResource.resources_key: [self.matrix]}) ] FakeResource.name_attribute = None @@ -1375,7 +1374,7 @@ class TestFind(base.TestCase): def test_nada_not_ignored(self): self.mock_get.side_effect = [ - ksa_exceptions.http.NotFound(), + exceptions.NotFoundException(), FakeResponse({FakeResource.resources_key: []}) ] @@ -1451,7 +1450,7 @@ class TestWaitForDelete(base.TestCase): sot.get = mock.Mock() sot.get.side_effect = [ sot, - ksa_exceptions.http.NotFound()] + exceptions.NotFoundException()] self.assertEqual(sot, resource.wait_for_delete(sess, sot, 1, 2)) diff --git a/openstack/tests/unit/test_session.py b/openstack/tests/unit/test_session.py index b5a4018c4..60ebad79c 100644 --- a/openstack/tests/unit/test_session.py +++ b/openstack/tests/unit/test_session.py @@ -10,8 +10,12 @@ # License for the specific language governing permissions and limitations # under the License. +import mock import testtools +from keystoneauth1 import exceptions as _exceptions + +from openstack import exceptions from openstack.image import image_service from openstack import session @@ -41,3 +45,43 @@ class TestSession(testtools.TestCase): def test_user_agent_set(self): sot = session.Session(None, user_agent="testing/123") self.assertTrue(sot.user_agent.startswith("testing/123 openstacksdk")) + + def test_map_exceptions_not_found_exception(self): + ksa_exc = _exceptions.HttpError(message="test", http_status=404) + func = mock.Mock(side_effect=ksa_exc) + + os_exc = self.assertRaises( + exceptions.NotFoundException, session.map_exceptions(func)) + self.assertIsInstance(os_exc, exceptions.NotFoundException) + self.assertEqual(ksa_exc.message, os_exc.message) + self.assertEqual(ksa_exc.http_status, os_exc.http_status) + self.assertEqual(ksa_exc, os_exc.cause) + + def test_map_exceptions_http_exception(self): + ksa_exc = _exceptions.HttpError(message="test", http_status=400) + func = mock.Mock(side_effect=ksa_exc) + + os_exc = self.assertRaises( + exceptions.HttpException, session.map_exceptions(func)) + self.assertIsInstance(os_exc, exceptions.HttpException) + self.assertEqual(ksa_exc.message, os_exc.message) + self.assertEqual(ksa_exc.http_status, os_exc.http_status) + self.assertEqual(ksa_exc, os_exc.cause) + + def test_map_exceptions_sdk_exception_1(self): + ksa_exc = _exceptions.ClientException() + func = mock.Mock(side_effect=ksa_exc) + + os_exc = self.assertRaises( + exceptions.SDKException, session.map_exceptions(func)) + self.assertIsInstance(os_exc, exceptions.SDKException) + self.assertEqual(ksa_exc, os_exc.cause) + + def test_map_exceptions_sdk_exception_2(self): + ksa_exc = _exceptions.VersionNotAvailable() + func = mock.Mock(side_effect=ksa_exc) + + os_exc = self.assertRaises( + exceptions.SDKException, session.map_exceptions(func)) + self.assertIsInstance(os_exc, exceptions.SDKException) + self.assertEqual(ksa_exc, os_exc.cause)