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

On 07/05/2016 04:30 PM, Lucio Correia wrote: > * Ignore empty messages > * Simplify exception message > > Signed-off-by: Lucio Correia <luciojhc(a)linux.vnet.ibm.com&gt; > --- > 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)))