Re: [Kimchi-devel] [PATCH 3/5] AsyncTask: Improve continuous status feedback

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(a)linux.vnet.ibm.com&gt; > --- > 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) _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel