[Kimchi-devel] [PATCH 3/5] AsyncTask: Improve continuous status feedback
Aline Manera
alinefm at linux.vnet.ibm.com
Wed Oct 29 16:42:51 UTC 2014
On 10/29/2014 02:14 PM, Aline Manera wrote:
>
> 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 at 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.
As the other patches does not depend on this one I will apply then and
wait for the next version of this.
>
>> 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 at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
More information about the Kimchi-devel
mailing list