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

[Zhou Zheng Sheng]

on 2014/10/30 00:14, 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.
> 

Thank you Aline. I just sent a new version according to your comments.