openstack/mistral
Code Issues Proposed changes

Add/Fix all error handling mechanism on REST API

Browse Source 
* Refactor error handling
 * add tests for correct error codes

Closes bug: #1320136
Co-Authored-By: Angus Salkeld <angus.salkeld@rackspace.com>
Change-Id: Ia33e19cab4879164acb7c8268c9c657f4196df45
This commit is contained in:
Angus Salkeld 2014-05-28 12:38:40 +10:00
parent 1e821db34b
commit 1ceb4b1e8a
11 changed files with 206 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
mistral/api/controllers/v1/execution.py
View File

@ -18,15 +18,14 @@ import json
import pecan
from pecan import rest
from pecan import abort
from wsme import types as wtypes
import wsmeext.pecan as wsme_pecan
from mistral import exceptions as ex
from mistral.api.controllers.v1 import task
from mistral.openstack.common import log as logging
from mistral.api.controllers import resource
from mistral.db import api as db_api
from mistral.utils import rest_utils
LOG = logging.getLogger(__name__)
@ -75,17 +74,15 @@ class ExecutionsController(rest.RestController):
tasks = task.TasksController()
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Execution, wtypes.text, wtypes.text)
def get(self, workbook_name, id):
LOG.debug("Fetch execution [workbook_name=%s, id=%s]" %
(workbook_name, id))
#TODO (nmakhotkin) This should be refactored later
try:
values = db_api.execution_get(workbook_name, id)
return Execution.from_dict(values)
except ex.NotFoundException as e:
abort(404, e.message)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Execution, wtypes.text, wtypes.text, body=Execution)
def put(self, workbook_name, id, execution):
LOG.debug("Update execution [workbook_name=%s, id=%s, execution=%s]" %
@ -97,13 +94,13 @@ class ExecutionsController(rest.RestController):
return Execution.from_dict(values)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Execution, wtypes.text, body=Execution,
status_code=201)
def post(self, workbook_name, execution):
LOG.debug("Create execution [workbook_name=%s, execution=%s]" %
(workbook_name, execution))
try:
context = None
if execution.context:
context = json.loads(execution.context)
@ -112,20 +109,15 @@ class ExecutionsController(rest.RestController):
values = engine.start_workflow_execution(execution.workbook_name,
execution.task,
context)
except ex.MistralException as e:
#TODO(nmakhotkin) we should use thing such a decorator here
abort(400, e.message)
return Execution.from_dict(values)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(None, wtypes.text, wtypes.text, status_code=204)
def delete(self, workbook_name, id):
LOG.debug("Delete execution [workbook_name=%s, id=%s]" %
(workbook_name, id))
try:
db_api.execution_delete(workbook_name, id)
except ex.NotFoundException as e:
abort(404, e.message)
@wsme_pecan.wsexpose(Executions, wtypes.text)
def get_all(self, workbook_name):

9
mistral/api/controllers/v1/listener.py
View File

@ -15,13 +15,13 @@
# limitations under the License.
from pecan import rest
from pecan import abort
from wsme import types as wtypes
import wsmeext.pecan as wsme_pecan
from mistral.openstack.common import log as logging
from mistral.api.controllers import resource
from mistral.db import api as db_api
from mistral.utils import rest_utils
LOG = logging.getLogger(__name__)
@ -58,16 +58,13 @@ class Listeners(resource.Resource):
class ListenersController(rest.RestController):
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Listener, wtypes.text, wtypes.text)
def get(self, workbook_name, id):
LOG.debug("Fetch listener [workbook_name=%s, id=%s]" %
(workbook_name, id))
values = db_api.listener_get(workbook_name, id)
if not values:
abort(404)
else:
return Listener.from_dict(values)
@wsme_pecan.wsexpose(Listener, wtypes.text, wtypes.text, body=Listener)
@ -79,6 +76,7 @@ class ListenersController(rest.RestController):
return Listener.from_dict(values)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Listener, wtypes.text, body=Listener, status_code=201)
def post(self, workbook_name, listener):
LOG.debug("Create listener [workbook_name=%s, listener=%s]" %
@ -88,6 +86,7 @@ class ListenersController(rest.RestController):
return Listener.from_dict(values)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(None, wtypes.text, wtypes.text, status_code=204)
def delete(self, workbook_name, id):
LOG.debug("Delete listener [workbook_name=%s, id=%s]" %

8
mistral/api/controllers/v1/task.py
View File

@ -16,13 +16,13 @@
import pecan
from pecan import rest
from pecan import abort
from wsme import types as wtypes
import wsmeext.pecan as wsme_pecan
from mistral.openstack.common import log as logging
from mistral.api.controllers import resource
from mistral.db import api as db_api
from mistral.utils import rest_utils
LOG = logging.getLogger(__name__)
@ -48,18 +48,16 @@ class Tasks(resource.Resource):
class TasksController(rest.RestController):
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Task, wtypes.text, wtypes.text, wtypes.text)
def get(self, workbook_name, execution_id, id):
LOG.debug("Fetch task [workbook_name=%s, execution_id=%s, id=%s]" %
(workbook_name, execution_id, id))
values = db_api.task_get(workbook_name, execution_id, id)
if not values:
abort(404)
return Task.from_dict(values)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Task, wtypes.text, wtypes.text, wtypes.text,
body=Task)
def put(self, workbook_name, execution_id, id, task):

18
mistral/api/controllers/v1/workbook.py
View File

@ -15,16 +15,15 @@
# limitations under the License.
from pecan import rest
from pecan import abort
from wsme import types as wtypes
import wsmeext.pecan as wsme_pecan
from mistral import exceptions as ex
from mistral.api.controllers.v1 import workbook_definition
from mistral.api.controllers.v1 import listener
from mistral.api.controllers.v1 import execution
from mistral.api.controllers import resource
from mistral.services import workbooks
from mistral.utils import rest_utils
from mistral.openstack.common import log as logging
from mistral.db import api as db_api
@ -51,17 +50,15 @@ class WorkbooksController(rest.RestController):
listeners = listener.ListenersController()
executions = execution.ExecutionsController()
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Workbook, wtypes.text)
def get(self, name):
LOG.debug("Fetch workbook [name=%s]" % name)
#TODO (nmakhotkin) This should be refactored later
try:
values = db_api.workbook_get(name)
return Workbook.from_dict(values)
except ex.NotFoundException as e:
abort(404, e.message)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Workbook, wtypes.text, body=Workbook)
def put(self, name, workbook):
LOG.debug("Update workbook [name=%s, workbook=%s]" % (name, workbook))
@ -69,23 +66,18 @@ class WorkbooksController(rest.RestController):
return Workbook.from_dict(db_api.workbook_update(name,
workbook.to_dict()))
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(Workbook, body=Workbook, status_code=201)
def post(self, workbook):
LOG.debug("Create workbook [workbook=%s]" % workbook)
try:
wb = workbooks.create_workbook(workbook.to_dict())
return Workbook.from_dict(wb)
except ex.MistralException as e:
#TODO(nmakhotkin) we should use thing such a decorator here
abort(400, e.message)
@rest_utils.wrap_wsme_controller_exception
@wsme_pecan.wsexpose(None, wtypes.text, status_code=204)
def delete(self, name):
LOG.debug("Delete workbook [name=%s]" % name)
try:
db_api.workbook_delete(name)
except ex.NotFoundException as e:
abort(404, e.message)
@wsme_pecan.wsexpose(Workbooks)
def get_all(self):

10
mistral/api/controllers/v1/workbook_definition.py
View File

@ -14,38 +14,34 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from pecan import abort
from pecan import rest
from pecan import expose
from pecan import request
import six
from mistral import exceptions as exc
from mistral.openstack.common import log as logging
from mistral.db import api as db_api
from mistral.services import scheduler
from mistral.utils import rest_utils
LOG = logging.getLogger(__name__)
class WorkbookDefinitionController(rest.RestController):
@rest_utils.wrap_pecan_controller_exception
@expose()
def get(self, workbook_name):
LOG.debug("Fetch workbook definition [workbook_name=%s]" %
workbook_name)
return db_api.workbook_definition_get(workbook_name)
@rest_utils.wrap_pecan_controller_exception
@expose(content_type="text/plain")
def put(self, workbook_name):
text = request.text
LOG.debug("Update workbook definition [workbook_name=%s, text=%s]" %
(workbook_name, text))
try:
wb = db_api.workbook_definition_put(workbook_name, text)
scheduler.create_associated_triggers(wb)
return wb['definition']
except exc.MistralException as e:
abort(400, six.text_type(e))

8
mistral/exceptions.py
View File

@ -26,6 +26,14 @@ class MistralException(ex.Error):
message = "An unknown exception occurred"
http_code = 500
@property
def code(self):
"""This is here for webob to read.
https://github.com/Pylons/webob/blob/master/webob/exc.py
"""
return self.http_code
def __str__(self):
return self.message

24
mistral/tests/api/v1/controllers/test_executions.py
View File

@ -62,10 +62,11 @@ class TestExecutionsController(base.FunctionalTest):
self.assertDictEqual(EXECS[0], canonize(resp.json))
@mock.patch.object(db_api, 'execution_get',
mock.MagicMock(side_effect=ex.NotFoundException))
mock.MagicMock(side_effect=ex.NotFoundException()))
def test_get_empty(self):
self.assertNotFound('/v1/workbooks/my_workbook/executions/123')
resp = self.app.get('/v1/workbooks/my_workbook/executions/123',
expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, 'execution_update',
mock.MagicMock(return_value=UPDATED_EXEC))
@ -76,6 +77,15 @@ class TestExecutionsController(base.FunctionalTest):
self.assertEqual(resp.status_int, 200)
self.assertDictEqual(UPDATED_EXEC, canonize(resp.json))
@mock.patch.object(db_api, 'execution_update',
mock.MagicMock(
side_effect=ex.NotFoundException()))
def test_put_not_found(self):
resp = self.app.put_json('/v1/workbooks/my_workbook/executions/123',
dict(state='STOPPED'), expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(engine.EngineClient, 'start_workflow_execution',
mock.MagicMock(return_value=EXECS[0]))
def test_post(self):
@ -102,6 +112,14 @@ class TestExecutionsController(base.FunctionalTest):
self.assertEqual(resp.status_int, 204)
@mock.patch.object(db_api, 'execution_delete',
mock.MagicMock(side_effect=ex.NotFoundException))
def test_delete_not_found(self):
resp = self.app.delete('/v1/workbooks/my_workbook/executions/123',
expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, 'executions_get',
mock.MagicMock(return_value=EXECS))
def test_get_all(self):

31
mistral/tests/api/v1/controllers/test_listeners.py
View File

@ -16,11 +16,10 @@
import mock
from mistral import exceptions
from mistral.tests.api import base
from mistral.db import api as db_api
# TODO: later we need additional tests verifying all the errors etc.
LISTENERS = [
{
'id': "1",
@ -43,6 +42,15 @@ class TestListenersController(base.FunctionalTest):
self.assertEqual(resp.status_int, 200)
self.assertDictEqual(LISTENERS[0], resp.json)
@mock.patch.object(db_api, "listener_get",
mock.MagicMock(
side_effect=exceptions.NotFoundException()))
def test_get_not_found(self):
resp = self.app.get('/v1/workbooks/my_workbook/listeners/1',
expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, "listener_update",
mock.MagicMock(return_value=UPDATED_LSNR))
def test_put(self):
@ -52,6 +60,16 @@ class TestListenersController(base.FunctionalTest):
self.assertEqual(resp.status_int, 200)
self.assertDictEqual(UPDATED_LSNR, resp.json)
@mock.patch.object(db_api, "listener_update",
mock.MagicMock(
side_effect=exceptions.NotFoundException()))
def test_put_not_found(self):
resp = self.app.put_json('/v1/workbooks/my_workbook/listeners/1',
dict(description='new description'),
expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, "listener_create",
mock.MagicMock(return_value=LISTENERS[0]))
def test_post(self):
@ -68,6 +86,15 @@ class TestListenersController(base.FunctionalTest):
self.assertEqual(resp.status_int, 204)
@mock.patch.object(db_api, "listener_delete",
mock.MagicMock(
side_effect=exceptions.NotFoundException()))
def test_delete_not_found(self):
resp = self.app.delete('/v1/workbooks/my_workbook/listeners/1',
expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, "listeners_get",
mock.MagicMock(return_value=LISTENERS))
def test_get_all(self):

24
mistral/tests/api/v1/controllers/test_workbook_definition.py
View File

@ -16,11 +16,10 @@
import mock
from mistral import exceptions
from mistral.tests.api import base
from mistral.db import api as db_api
# TODO: later we need additional tests verifying all the errors etc.
DEFINITION = "my definition"
NEW_DEFINITION = """
@ -57,6 +56,16 @@ class TestWorkbookDefinitionController(base.FunctionalTest):
self.assertEqual(resp.status_int, 200)
self.assertEqual(DEFINITION, resp.text)
@mock.patch.object(db_api, "workbook_definition_get",
mock.MagicMock(
side_effect=exceptions.NotFoundException()))
def test_get_not_found(self):
resp = self.app.get('/v1/workbooks/my_workbook/definition',
headers={"Content-Type": "text/plain"},
expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, "workbook_definition_put",
mock.MagicMock(return_value={
'name': 'my_workbook',
@ -76,3 +85,14 @@ class TestWorkbookDefinitionController(base.FunctionalTest):
self.assertEqual(triggers[0]['name'], 'create-vms')
self.assertEqual(triggers[0]['pattern'], '* * * * *')
self.assertEqual(triggers[0]['workbook_name'], 'my_workbook')
@mock.patch.object(db_api, "workbook_definition_put",
mock.MagicMock(
side_effect=exceptions.NotFoundException()))
def test_put_not_found(self):
resp = self.app.put('/v1/workbooks/my_workbook/definition',
NEW_DEFINITION,
headers={"Content-Type": "text/plain"},
expect_errors=True)
self.assertEqual(resp.status_int, 404)

37
mistral/tests/api/v1/controllers/test_workbooks.py
View File

@ -16,11 +16,10 @@
import mock
from mistral import exceptions
from mistral.tests.api import base
from mistral.db import api as db_api
# TODO: later we need additional tests verifying all the errors etc.
WORKBOOKS = [
{
'name': "my_workbook",
@ -42,6 +41,13 @@ class TestWorkbooksController(base.FunctionalTest):
self.assertEqual(resp.status_int, 200)
self.assertDictEqual(WORKBOOKS[0], resp.json)
@mock.patch.object(db_api, "workbook_get",
mock.MagicMock(
side_effect=exceptions.NotFoundException()))
def test_get_not_found(self):
resp = self.app.get('/v1/workbooks/dev_null', expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, "workbook_update",
mock.MagicMock(return_value=UPDATED_WORKBOOK))
def test_put(self):
@ -51,6 +57,15 @@ class TestWorkbooksController(base.FunctionalTest):
self.assertEqual(resp.status_int, 200)
self.assertDictEqual(UPDATED_WORKBOOK, resp.json)
@mock.patch.object(db_api, "workbook_update",
mock.MagicMock(
side_effect=exceptions.NotFoundException()))
def test_put_not_found(self):
resp = self.app.put_json('/v1/workbooks/my_workbook',
dict(description='new description'),
expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, "workbook_create",
mock.MagicMock(return_value=WORKBOOKS[0]))
@mock.patch("mistral.services.trusts.create_trust",
@ -61,6 +76,16 @@ class TestWorkbooksController(base.FunctionalTest):
self.assertEqual(resp.status_int, 201)
self.assertDictEqual(WORKBOOKS[0], resp.json)
@mock.patch.object(db_api, "workbook_create",
mock.MagicMock(side_effect=exceptions.DBDuplicateEntry))
@mock.patch("mistral.services.trusts.create_trust",
mock.MagicMock(return_value=WORKBOOKS[0]))
def test_post_dup(self):
resp = self.app.post_json('/v1/workbooks', WORKBOOKS[0],
expect_errors=True)
self.assertEqual(resp.status_int, 409)
@mock.patch.object(db_api, "workbook_delete",
mock.MagicMock(return_value=None))
def test_delete(self):
@ -68,6 +93,14 @@ class TestWorkbooksController(base.FunctionalTest):
self.assertEqual(resp.status_int, 204)
@mock.patch.object(db_api, "workbook_delete",
mock.MagicMock(
side_effect=exceptions.NotFoundException()))
def test_delete_not_found(self):
resp = self.app.delete('/v1/workbooks/my_workbook', expect_errors=True)
self.assertEqual(resp.status_int, 404)
@mock.patch.object(db_api, "workbooks_get",
mock.MagicMock(return_value=WORKBOOKS))
def test_get_all(self):

52
mistral/utils/rest_utils.py Normal file
View File

@ -0,0 +1,52 @@
# -*- coding: utf-8 -*-
#
# Copyright 2014 - Mirantis, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import functools
import pecan
import six
from wsme import exc
from mistral import exceptions as ex
def wrap_wsme_controller_exception(func):
"""This decorator wraps controllers method to manage wsme exceptions:
In case of expected error it aborts the request with specific status code.
"""
@functools.wraps(func)
def wrapped(*args, **kwargs):
try:
return func(*args, **kwargs)
except ex.MistralException as excp:
pecan.response.translatable_error = excp
raise exc.ClientSideError(msg=six.text_type(excp),
status_code=excp.http_code)
return wrapped
def wrap_pecan_controller_exception(func):
"""This decorator wraps controllers method to manage pecan exceptions:
In case of expected error it aborts the request with specific status code.
"""
@functools.wraps(func)
def wrapped(*args, **kwargs):
try:
return func(*args, **kwargs)
except ex.MistralException as excp:
pecan.response.translatable_error = excp
pecan.abort(excp.http_code, six.text_type(excp))
return wrapped