[Kimchi-devel] [PATCH] [Wok] Use callback instead of log file for run_command output

[Lucio Correia]

On 05-07-2016 18:10, Aline Manera wrote:
>
>
> On 07/05/2016 04:30 PM, Lucio Correia wrote:
>>   * Ignore empty messages
>>   * Simplify exception message
>>
>> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
>> ---
>>   src/wok/asynctask.py | 13 +++++--------
>>   src/wok/utils.py     | 41 +++++++++++++----------------------------
>>   2 files changed, 18 insertions(+), 36 deletions(-)
>>
>> diff --git a/src/wok/asynctask.py b/src/wok/asynctask.py
>> index ec3fbc1..fb614a2 100644
>> --- a/src/wok/asynctask.py
>> +++ b/src/wok/asynctask.py
>> @@ -46,15 +46,12 @@ class AsyncTask(object):
>>           self.thread.start()
>>
>>       def _status_cb(self, message, success=None):
>> -        if success is None:
>> -            self.message = message
>> -            self._save_helper()
>> -            return
>> -
>>           if success is not None:
>>               self.status = 'finished' if success else 'failed'
>> -        self.message = message
>> -        self._save_helper()
>> +
>> +        if message.strip():
>> +            self.message = message
>> +            self._save_helper()
>>
>>       def _save_helper(self):
>>           obj = {}
>> @@ -73,4 +70,4 @@ class AsyncTask(object):
>>           except Exception, e:
>>               cherrypy.log.error_log.error("Error in async_task %s " %
>> self.id)
>>               cherrypy.log.error_log.error(traceback.format_exc())
>> -            cb("Unexpected exception: %s" % e.message, False)
>> +            cb(e.message, False)
>> diff --git a/src/wok/utils.py b/src/wok/utils.py
>> index 4b3b0ac..d532f96 100644
>> --- a/src/wok/utils.py
>> +++ b/src/wok/utils.py
>> @@ -175,16 +175,21 @@ def import_module(module_name, class_name=''):
>>       return __import__(module_name, globals(), locals(), [class_name])
>>
>>
>> -def run_command(cmd, timeout=None, silent=False, tee=None,
>> -                env_vars=None):
>> +def run_command(cmd, timeout=None, silent=False, out_cb=None,
>> env_vars=None):
>>       """
>>       cmd is a sequence of command arguments.
>>       timeout is a float number in seconds.
>>       timeout default value is None, means command run without timeout.
>>       silent is bool, it will log errors using debug handler not error.
>>       silent default value is False.
>> -    tee is a file path to store the output of the command, like 'tee'
>> command.
>> -    tee default value is None, means output will not be logged.
>> +    out_cb is a callback that receives the whole command output every
>> time a
>> +    new line is thrown by command. Default value is None, meaning
>> that whole
>> +    output will be returned at the end of execution.
>> +
>> +    Returns a tuple (out, error, returncode) where:
>> +    out is the output thrown by command
>> +    error is an error message if applicable
>> +    returncode is an integer equal to the result of command execution
>>       """
>>       # subprocess.kill() can leave descendants running
>>       # and halting the execution. Using psutil to
>> @@ -202,24 +207,6 @@ def run_command(cmd, timeout=None, silent=False,
>> tee=None,
>>           else:
>>               timeout_flag[0] = True
>>
>> -    # function to append the given msg into the log_file
>> -    def tee_log(msg=None, log_file=None):
>> -        if (msg is None) or (log_file is None):
>> -            return
>> -
>> -        try:
>> -            f = open(log_file, 'a')
>> -        except IOError as e:
>> -            msg = "Failed to open file %s: " % log_file
>> -            wok_log.error("%s %s", msg, e)
>> -            return
>> -        msg += '\n'
>> -        try:
>> -            f.write(msg)
>> -        except TypeError:
>> -            f.write(msg.encode('utf_8'))
>> -        f.close()
>> -
>>       proc = None
>>       timer = None
>>       timeout_flag = [False]
>> @@ -239,9 +226,7 @@ def run_command(cmd, timeout=None, silent=False,
>> tee=None,
>>               timer.start()
>>
>>           wok_log.debug("Run command: '%s'", " ".join(cmd))
>> -        if tee is not None:
>> -            if os.path.exists(tee):
>> -                os.remove(tee)
>
>> +        if out_cb is not None:
>>               output = []
>>               while True:
>>                   line = ""
>> @@ -260,7 +245,7 @@ def run_command(cmd, timeout=None, silent=False,
>> tee=None,
>>                   if not line:
>>                       break
>>                   line = line.rstrip('\n\r')
>> -                tee_log(line, tee)
>> +                out_cb(''.join(output))
>
> Question: can't we record the message (ie, line value) as is? do we
> really need to append to a list (output) and the use .join()?

Today we cannot because UI tracks progress using AsyncTask 'message' 
field. Since they run in different frequencies, this way we make sure 
the complete command output is shown (no line is missed, for example).

It's already in my plans to do an RFC for improving AsyncTask to work 
better for this case.

>
> Also, the line
>
> line = line.rstrip(...)
>
> can be removed as it is not in use anymore.

Yes, I will send V2.


>
>
>>               out = ''.join(output)
>>               error = proc.stderr.read()
>>               returncode = proc.poll()
>> @@ -281,8 +266,8 @@ def run_command(cmd, timeout=None, silent=False,
>> tee=None,
>>
>>               else:
>>                   wok_log.error(msg)
>> -                if tee is not None:
>> -                    tee_log(msg, tee)
>> +                if out_cb is not None:
>> +                    out_cb(msg)
>>           elif error:
>>               wok_log.debug("error: %s returned from cmd: %s",
>>                             decode_value(error), decode_value('
>> '.join(cmd)))
>


-- 
Lucio Correia
Software Engineer
IBM LTC Brazil