On 10/28/2014 04:48 AM, Zhou Zheng Sheng wrote:
One of the advantage of AsyncTask is that the background task can call "_status_cb()" multiple times to report the interim progress. However this is not properly implemented in "_status_cb()". The correct handling should be as follow.
cb(message, True) Means task finished succesfully.
cb(message, False) Means task failed.
cb(message) cb(message, None) Means task is ongoing and the invocation is to provide progress feedback.
The current implementation fails to distinguish "cb(message, False)" and "cb(message)". This patch fixes the problem and adds a new test case.
This patch also improves the "_wait_task()" methods in tests, adding a regular status report to allow developer know it's waiting for some task but not stuck on something. There is also two methods in tests that start a AsyncTask but do not wait it till finish, this will interfere with other tests, so the patch adds the missing "_wait_task()" invocation for them.
Signed-off-by: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> --- src/kimchi/asynctask.py | 4 ++-- tests/test_mockmodel.py | 14 ++++++++++++++ tests/test_model.py | 31 ++++++++++++++++++++++++++----- tests/test_rest.py | 7 +++++++ 4 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/src/kimchi/asynctask.py b/src/kimchi/asynctask.py index 99e7a64..f8e55c0 100644 --- a/src/kimchi/asynctask.py +++ b/src/kimchi/asynctask.py @@ -49,9 +49,9 @@ class AsyncTask(object): self._save_helper() return
- if success: + if success == True: self.status = 'finished' - else: + elif success ==False: self.status = 'failed' self.message = message self._save_helper() diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py index 8de7ce9..3158c48 100644 --- a/tests/test_mockmodel.py +++ b/tests/test_mockmodel.py @@ -27,6 +27,7 @@ import unittest import kimchi.mockmodel from utils import get_free_port, patch_auth, request, run_server from kimchi.control.base import Collection, Resource +from kimchi.utils import kimchi_log
test_server = None @@ -174,3 +175,16 @@ class MockModelTests(unittest.TestCase): task = model.host_swupdate() task_params = [u'id', u'message', u'status', u'target_uri'] self.assertEquals(sorted(task_params), sorted(task.keys())) + self._wait_task(model, task['id'])
+ + def _wait_task(self, model, taskid, timeout=5): + for i in range(0, timeout): + task_info = model.task_lookup(taskid) + if task_info['status'] == "running": + kimchi_log.info("Waiting task %s, message: %s", + task_info['id'], task_info['message']) + time.sleep(1) + else: + return + kimchi_log.error("Timeout while process long-run task, " + "try to increase timeout value.")
We could move it to tests/utils.py to avoid duplicating code when a Task needs to be watched.
diff --git a/tests/test_model.py b/tests/test_model.py index e407fe5..c0e8cb5 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -42,7 +42,7 @@ from kimchi.exception import InvalidParameter, NotFoundError, OperationFailed from kimchi.iscsi import TargetClient from kimchi.model import model from kimchi.rollbackcontext import RollbackContext -from kimchi.utils import add_task +from kimchi.utils import add_task, kimchi_log
invalid_repository_urls = ['www.fedora.org', # missing protocol @@ -285,8 +285,9 @@ class ModelTests(unittest.TestCase): 'capacity': 1024, 'allocation': 512, 'format': 'qcow2'} - inst.storagevolumes_create(pool, params) + task_id = inst.storagevolumes_create(pool, params)['id'] rollback.prependDefer(inst.storagevolume_delete, pool, vol) + self._wait_task(inst, task_id)
vm_name = 'kimchi-cdrom' params = {'name': 'test', 'disks': [], 'cdrom': self.kimchi_iso} @@ -1107,6 +1108,13 @@ class ModelTests(unittest.TestCase): except: cb("Exception raised", False)
+ def continuous_ops(cb, params): + cb("step 1 OK") + time.sleep(2) + cb("step 2 OK") + time.sleep(2) + cb("step 3 OK", params.get('result', True)) + inst = model.Model('test:///default', objstore_loc=self.tmp_store) taskid = add_task('', quick_op, inst.objstore, 'Hello') @@ -1131,6 +1139,12 @@ class ModelTests(unittest.TestCase): inst.task_lookup(taskid)['message']) self.assertEquals('failed', inst.task_lookup(taskid)['status'])
+ taskid = add_task('', continuous_ops, inst.objstore, + {'result': True}) + self.assertEquals('running', inst.task_lookup(taskid)['status']) + self._wait_task(inst, taskid, timeout=10) + self.assertEquals('finished', inst.task_lookup(taskid)['status']) + # This wrapper function is needed due to the new backend messaging in # vm model. vm_poweroff and vm_delete raise exception if vm is not found. # These functions are called after vm has been deleted if test finishes @@ -1253,9 +1267,16 @@ class ModelTests(unittest.TestCase): raise e
def _wait_task(self, model, taskid, timeout=5): - for i in range(0, timeout): - if model.task_lookup(taskid)['status'] == 'running': - time.sleep(1) + for i in range(0, timeout): + task_info = model.task_lookup(taskid) + if task_info['status'] == "running": + kimchi_log.info("Waiting task %s, message: %s", + taskid, task_info['message']) + time.sleep(1) + else: + return + kimchi_log.error("Timeout while process long-run task, " + "try to increase timeout value.")
def test_get_distros(self): inst = model.Model('test:///default', diff --git a/tests/test_rest.py b/tests/test_rest.py index 8de0a9c..649790b 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -37,6 +37,7 @@ import kimchi.mockmodel import kimchi.server from kimchi.config import paths from kimchi.rollbackcontext import RollbackContext +from kimchi.utils import kimchi_log from utils import get_free_port, patch_auth, request from utils import run_server
@@ -1535,7 +1536,13 @@ class RestTests(unittest.TestCase): for i in range(0, timeout): task = json.loads(self.request('/tasks/%s' % taskid).read()) if task['status'] == 'running': + kimchi_log.info("Waiting task %d, message: %s", + taskid, task['message']) time.sleep(1) + else: + return + kimchi_log.error("Timeout while process long-run task, " + "try to increase timeout value.")
def test_tasks(self): id1 = model.add_task('/tasks/1', self._async_op)