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

[Aline Manera]

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()?

Also, the line

line = line.rstrip(...)

can be removed as it is not in use anymore.


>               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)))