[PATCH V1] add a synchronous function with timeout to execute command
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> We need a common function to execute shell command. We also need timeout when execute shell command. A threading.Timer is used to send signal.SIGKILL to kill the command when timeout. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/utils.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index af245c6..94097ad 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -23,6 +23,7 @@ import cherrypy import os +import subprocess import urllib2 @@ -30,6 +31,7 @@ from cherrypy.lib.reprconf import Parser from kimchi import config +from threading import Timer kimchi_log = cherrypy.log.error_log @@ -96,3 +98,40 @@ def check_url_path(path): return False return True + + +def run_command(cmd, timeout=None): + def kill_proc(proc, timeout_flag): + timeout_flag[0] = True + proc.kill() + + proc = None + timer = None + timeout_flag = [False] + + try: + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + if timeout is not None: + timer = Timer(timeout, kill_proc, [proc, timeout_flag]) + timer.start() + + out, error = proc.communicate() + kimchi_log.debug("Run command '%s'", " ".join(cmd)) + + if out or error: + kimchi_log.debug("out:\n %s\nerror:\n %s", out, error) + + return out, error, proc.returncode, timeout_flag[0] + except Exception as e: + msg = "Failed to run command: %s." % " ".join(cmd) + msg = msg if proc is None else msg + "\n error code: %s." + kimchi_log.error("%s\n %s", msg, e) + + if proc: + return out, error, proc.returncode, timeout_flag[0] + else: + return None, None, None, timeout_flag[0] + finally: + if timer and not timeout_flag[0]: + timer.cancel() -- 1.8.4.2
On 2014年01月10日 09:00, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
We need a common function to execute shell command. We also need timeout when execute shell command.
A threading.Timer is used to send signal.SIGKILL to kill the command when timeout.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/utils.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index af245c6..94097ad 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -23,6 +23,7 @@
import cherrypy import os +import subprocess import urllib2
@@ -30,6 +31,7 @@ from cherrypy.lib.reprconf import Parser
from kimchi import config +from threading import Timer
kimchi_log = cherrypy.log.error_log @@ -96,3 +98,40 @@ def check_url_path(path): return False
return True + + +def run_command(cmd, timeout=None): + def kill_proc(proc, timeout_flag): + timeout_flag[0] = True + proc.kill() + + proc = None + timer = None + timeout_flag = [False] + + try: + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + if timeout is not None: + timer = Timer(timeout, kill_proc, [proc, timeout_flag]) + timer.start() I like the idea of using timer, I think the only thing we need to take care is check timeout happens and report a specific error. Or when we kill the proc and it is blocked, the proc.communicate() error will not be 'timeout happens' + + out, error = proc.communicate() + kimchi_log.debug("Run command '%s'", " ".join(cmd)) + + if out or error: + kimchi_log.debug("out:\n %s\nerror:\n %s", out, error) + + return out, error, proc.returncode, timeout_flag[0] + except Exception as e: + msg = "Failed to run command: %s." % " ".join(cmd) + msg = msg if proc is None else msg + "\n error code: %s." + kimchi_log.error("%s\n %s", msg, e) + + if proc: + return out, error, proc.returncode, timeout_flag[0] + else: + return None, None, None, timeout_flag[0] + finally: + if timer and not timeout_flag[0]: + timer.cancel()
On 01/10/2014 04:16 PM, Royce Lv wrote:
On 2014年01月10日 09:00, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
We need a common function to execute shell command. We also need timeout when execute shell command.
A threading.Timer is used to send signal.SIGKILL to kill the command when timeout.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/utils.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index af245c6..94097ad 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -23,6 +23,7 @@
import cherrypy import os +import subprocess import urllib2
@@ -30,6 +31,7 @@ from cherrypy.lib.reprconf import Parser
from kimchi import config +from threading import Timer
kimchi_log = cherrypy.log.error_log @@ -96,3 +98,40 @@ def check_url_path(path): return False
return True + + +def run_command(cmd, timeout=None): + def kill_proc(proc, timeout_flag): + timeout_flag[0] = True + proc.kill() + + proc = None + timer = None + timeout_flag = [False] + + try: + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + if timeout is not None: + timer = Timer(timeout, kill_proc, [proc, timeout_flag]) + timer.start() I like the idea of using timer, I think the only thing we need to take care is check timeout happens and report a specific error. Or when we kill the proc and it is blocked, the proc.communicate() error will not be 'timeout happens'
when we kill the proc, the proc.returncode will be -9 and timeout_flag[0] is True. so we can log the timeout if proc.returncode == -9 and timeout_flag[0] is True: kimchi_log.error("process is killed by signal.SIGKILL for timeout %s seconds", timeout) or raise a TimeoutError: we can define a new Exception TimeoutError. if proc.returncode == -9 and timeout_flag[0] is True: raise TimeoutError("process is killed by signal.SIGKILL for timeout %s seconds" % timeout)
+ + out, error = proc.communicate() + kimchi_log.debug("Run command '%s'", " ".join(cmd)) + + if out or error: + kimchi_log.debug("out:\n %s\nerror:\n %s", out, error) + + return out, error, proc.returncode, timeout_flag[0] + except Exception as e: + msg = "Failed to run command: %s." % " ".join(cmd) + msg = msg if proc is None else msg + "\n error code: %s." + kimchi_log.error("%s\n %s", msg, e) + + if proc: + return out, error, proc.returncode, timeout_flag[0] + else: + return None, None, None, timeout_flag[0] + finally: + if timer and not timeout_flag[0]: + timer.cancel()
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
How about just check timeout by ”proc.returncode == -9“, not sure we can kill the subprocess manually. The follow is an Example: /import subprocess// //from threading import Timer// // //def run_command(cmd, timeout=None):// // proc = None// // timeout_flag = False// // // try:// // proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,// // stderr=subprocess.PIPE)// // if timeout is not None:// // timer = Timer(timeout, lambda proc: proc.kill(), [proc])// // timer.start()// // // out, error = proc.communicate()// // print "Run command '%s'" % " ".join(cmd)// // // if out or error:// // print "out:\n %s\nerror:\n %s" % (out, error)// // if proc.returncode == -9:// // timeout_flag = True// // print "process is killed by signal.SIGKILL for timeout %s seconds" % timeout// // if timeout is not None:// // timer.cancel()// // return out, error, proc.returncode, timeout_flag// // except Exception as e:// // msg = "Failed to run command: %s." % " ".join(cmd)// // print "%s\n %s" % (msg, e)// // return None, None, None, timeout_flag// // / On 01/10/2014 04:16 PM, Royce Lv wrote:
On 2014年01月10日 09:00, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
We need a common function to execute shell command. We also need timeout when execute shell command.
A threading.Timer is used to send signal.SIGKILL to kill the command when timeout.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/utils.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index af245c6..94097ad 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -23,6 +23,7 @@
import cherrypy import os +import subprocess import urllib2
@@ -30,6 +31,7 @@ from cherrypy.lib.reprconf import Parser
from kimchi import config +from threading import Timer
kimchi_log = cherrypy.log.error_log @@ -96,3 +98,40 @@ def check_url_path(path): return False
return True + + +def run_command(cmd, timeout=None): + def kill_proc(proc, timeout_flag): + timeout_flag[0] = True + proc.kill() + + proc = None + timer = None + timeout_flag = [False] + + try: + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + if timeout is not None: + timer = Timer(timeout, kill_proc, [proc, timeout_flag]) + timer.start() I like the idea of using timer, I think the only thing we need to take care is check timeout happens and report a specific error. Or when we kill the proc and it is blocked, the proc.communicate() error will not be 'timeout happens' + + out, error = proc.communicate() + kimchi_log.debug("Run command '%s'", " ".join(cmd)) + + if out or error: + kimchi_log.debug("out:\n %s\nerror:\n %s", out, error) + + return out, error, proc.returncode, timeout_flag[0] + except Exception as e: + msg = "Failed to run command: %s." % " ".join(cmd) + msg = msg if proc is None else msg + "\n error code: %s." + kimchi_log.error("%s\n %s", msg, e) + + if proc: + return out, error, proc.returncode, timeout_flag[0] + else: + return None, None, None, timeout_flag[0] + finally: + if timer and not timeout_flag[0]: + timer.cancel()
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
How about just check timeout by ”proc.returncode == -9“, not sure we can kill the subprocess manually.
The follow is an Example:
/import subprocess// //from threading import Timer// // //def run_command(cmd, timeout=None):// // proc = None// // timeout_flag = False// // // try:// // proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,// // stderr=subprocess.PIPE)// // if timeout is not None:// // timer = Timer(timeout, lambda proc: proc.kill(), [proc])// // timer.start()// // // out, error = proc.communicate()// // print "Run command '%s'" % " ".join(cmd)// // // if out or error:// // print "out:\n %s\nerror:\n %s" % (out, error)// // if proc.returncode == -9:// // timeout_flag = True// // print "process is killed by signal.SIGKILL for timeout %s seconds" % timeout// // if timeout is not None:// // timer.cancel()// // return out, error, proc.returncode, timeout_flag// // except Exception as e:// // msg = "Failed to run command: %s." % " ".join(cmd)// // print "%s\n %s" % (msg, e)// // return None, None, None, timeout_flag// // / On 01/10/2014 04:16 PM, Royce Lv wrote:
On 2014年01月10日 09:00, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
We need a common function to execute shell command. We also need timeout when execute shell command.
A threading.Timer is used to send signal.SIGKILL to kill the command when timeout.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/utils.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index af245c6..94097ad 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -23,6 +23,7 @@
import cherrypy import os +import subprocess import urllib2
@@ -30,6 +31,7 @@ from cherrypy.lib.reprconf import Parser
from kimchi import config +from threading import Timer
kimchi_log = cherrypy.log.error_log @@ -96,3 +98,40 @@ def check_url_path(path): return False
return True + + +def run_command(cmd, timeout=None): + def kill_proc(proc, timeout_flag): + timeout_flag[0] = True + proc.kill() + + proc = None + timer = None + timeout_flag = [False] + + try: + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + if timeout is not None: + timer = Timer(timeout, kill_proc, [proc, timeout_flag]) + timer.start() I like the idea of using timer, I think the only thing we need to take care is check timeout happens and report a specific error. Or when we kill the proc and it is blocked, the proc.communicate() error will not be 'timeout happens' + + out, error = proc.communicate() + kimchi_log.debug("Run command '%s'", " ".join(cmd)) + + if out or error: + kimchi_log.debug("out:\n %s\nerror:\n %s", out, error) + + return out, error, proc.returncode, timeout_flag[0] + except Exception as e: + msg = "Failed to run command: %s." % " ".join(cmd) + msg = msg if proc is None else msg + "\n error code: %s." + kimchi_log.error("%s\n %s", msg, e) + + if proc: + return out, error, proc.returncode, timeout_flag[0] + else: + return None, None, None, timeout_flag[0] I like the idea raise Timeout Exception better, because when timeout occurs, the ordinary execution of cmd becomes invalid and we do not need to check the execute result any more. And outside of this helper function, I think using:
On 2014年01月10日 21:34, Sheldon wrote: try: run_cmd(cmd) except TimeoutException: ... Will help to separate the ordinary logic. What do you think?
+ finally: + if timer and not timeout_flag[0]: + timer.cancel()
-- Thanks and best regards!
Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
participants (3)
-
Royce Lv -
shaohef＠linux.vnet.ibm.com -
Sheldon