From c08999a59c8b2beb9489e712fe90fe1c731fa2d5 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Mon, 23 Apr 2018 12:55:25 +0200 Subject: [PATCH] mgr/dashboard: Improved Exception handling in Tasks * Set default status code to 400 * Added tests * Fixed globbing in `test_task.py` Signed-off-by: Sebastian Wagner --- qa/tasks/mgr/dashboard/test_rbd.py | 4 ++-- .../mgr/dashboard/controllers/__init__.py | 3 +-- .../mgr/dashboard/tests/test_exceptions.py | 23 +++++++++++++++---- src/pybind/mgr/dashboard/tests/test_task.py | 10 ++++---- src/pybind/mgr/dashboard/tools.py | 3 +++ 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/qa/tasks/mgr/dashboard/test_rbd.py b/qa/tasks/mgr/dashboard/test_rbd.py index 85ffa01a47c..73267d11dbd 100644 --- a/qa/tasks/mgr/dashboard/test_rbd.py +++ b/qa/tasks/mgr/dashboard/test_rbd.py @@ -316,8 +316,8 @@ class RbdTest(DashboardTestCase): def test_delete_non_existent_image(self): res = self.remove_image('rbd', 'i_dont_exist') - self.assertStatus(409) - self.assertEqual(res, {"errno": 2, "status": 409, "component": "rbd", + self.assertStatus(400) + self.assertEqual(res, {"errno": 2, "status": 400, "component": "rbd", "detail": "[errno 2] error removing image"}) def test_image_delete(self): diff --git a/src/pybind/mgr/dashboard/controllers/__init__.py b/src/pybind/mgr/dashboard/controllers/__init__.py index 3c47ef96c48..b0fdf02faae 100644 --- a/src/pybind/mgr/dashboard/controllers/__init__.py +++ b/src/pybind/mgr/dashboard/controllers/__init__.py @@ -319,7 +319,7 @@ class Task(object): if 'status' in task.ret_value: status = task.ret_value['status'] else: - status = 500 + status = getattr(ex, 'status', 500) cherrypy.response.status = status return task.ret_value raise ex @@ -327,7 +327,6 @@ class Task(object): cherrypy.response.status = 202 return {'name': self.name, 'metadata': md} return value - wrapper.__wrapped__ = func return wrapper diff --git a/src/pybind/mgr/dashboard/tests/test_exceptions.py b/src/pybind/mgr/dashboard/tests/test_exceptions.py index ae11192487b..78472022a4c 100644 --- a/src/pybind/mgr/dashboard/tests/test_exceptions.py +++ b/src/pybind/mgr/dashboard/tests/test_exceptions.py @@ -1,20 +1,23 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import +import time + import cherrypy import rados from ..services.ceph_service import SendCommandError -from ..controllers import RESTController, ApiController +from ..controllers import RESTController, ApiController, Task from .helper import ControllerTestCase from ..services.exception import handle_rados_error, handle_send_command_error, \ serialize_dashboard_exception -from ..tools import ViewCache, TaskManager +from ..tools import ViewCache, TaskManager, NotificationQueue # pylint: disable=W0613 @ApiController('foo') class FooResource(RESTController): + @cherrypy.expose @cherrypy.tools.json_out() @handle_rados_error('foo') @@ -43,7 +46,6 @@ class FooResource(RESTController): def vc_no_data(self): @ViewCache(timeout=0) def _no_data(): - import time time.sleep(0.2) _no_data() @@ -90,9 +92,10 @@ class Root(object): class RESTControllerTest(ControllerTestCase): - @classmethod def setup_server(cls): + NotificationQueue.start_queue() + TaskManager.init() cls.setup_controllers([FooResource]) def test_no_exception(self): @@ -145,6 +148,18 @@ class RESTControllerTest(ControllerTestCase): {'detail': '[errno -42] hi', 'code': "42", 'component': 'foo'} ) + def test_task_exception(self): + self._get('/foo/task_exception') + self.assertStatus(400) + self.assertJsonBody( + {'detail': '[errno -42] hi', 'code': "42", 'component': 'foo'} + ) + + self._get('/foo/wait_task_exception') + while self.jsonBody(): + time.sleep(0.5) + self._get('/foo/wait_task_exception') + def test_internal_server_error(self): self._get('/foo/internal_server_error') self.assertStatus(500) diff --git a/src/pybind/mgr/dashboard/tests/test_task.py b/src/pybind/mgr/dashboard/tests/test_task.py index 0547a00a43e..872deb6fa1e 100644 --- a/src/pybind/mgr/dashboard/tests/test_task.py +++ b/src/pybind/mgr/dashboard/tests/test_task.py @@ -183,7 +183,7 @@ class TaskTest(unittest.TestCase): state, result = task1.run('test5/task1', 0.5) self.assertEqual(state, TaskManager.VALUE_EXECUTING) self.assertIsNone(result) - ex_t, _ = TaskManager.list() + ex_t, _ = TaskManager.list('test5/*') self.assertEqual(len(ex_t), 1) self.assertEqual(ex_t[0].name, 'test5/task1') self.assertEqual(ex_t[0].progress, 30) @@ -199,12 +199,12 @@ class TaskTest(unittest.TestCase): self.assertEqual(task.progress, 60) task2.resume() self.wait_for_task('test5/task2') - ex_t, _ = TaskManager.list() + ex_t, _ = TaskManager.list('test5/*') self.assertEqual(len(ex_t), 1) self.assertEqual(ex_t[0].name, 'test5/task1') task1.resume() self.wait_for_task('test5/task1') - ex_t, _ = TaskManager.list() + ex_t, _ = TaskManager.list('test5/*') self.assertEqual(len(ex_t), 0) def test_task_idempotent(self): @@ -213,13 +213,13 @@ class TaskTest(unittest.TestCase): state, result = task1.run('test6/task1', 0.5) self.assertEqual(state, TaskManager.VALUE_EXECUTING) self.assertIsNone(result) - ex_t, _ = TaskManager.list() + ex_t, _ = TaskManager.list('test6/*') self.assertEqual(len(ex_t), 1) self.assertEqual(ex_t[0].name, 'test6/task1') state, result = task1_clone.run('test6/task1', 0.5) self.assertEqual(state, TaskManager.VALUE_EXECUTING) self.assertIsNone(result) - ex_t, _ = TaskManager.list() + ex_t, _ = TaskManager.list('test6/*') self.assertEqual(len(ex_t), 1) self.assertEqual(ex_t[0].name, 'test6/task1') task1.resume() diff --git a/src/pybind/mgr/dashboard/tools.py b/src/pybind/mgr/dashboard/tools.py index d1739e44eb0..8e8e536288e 100644 --- a/src/pybind/mgr/dashboard/tools.py +++ b/src/pybind/mgr/dashboard/tools.py @@ -585,6 +585,9 @@ class Task(object): return "Task(ns={}, md={})" \ .format(self.name, self.metadata) + def __repr__(self): + return str(self) + def _run(self): with self.lock: assert not self.running -- 2.39.5