CODEWITHSUNDEEP
pythonsubprocess
Reyhaneh
Reyhaneh

Reputation: 409

Check a condition while running subprocess

I am executing a command using subprocess.Popen(). I want to wait for the process to finish before executing the rest of my code but at the same time I would like to check the status of the produced files after 2 min of running the subprocess. If the size of file is zero then I want to stop the procedure. Currently my code is as below. Is there any smarter way to do this?

  def execute(command,outputfilename):
    process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
    start_time=time.time()
    Is_Working=False
    while True:
     process.poll()
     if not Is_Working:
         #allow 2 minutes for the process to create results
         if process.returncode == None and (time.time()-start_time)//60>1:
             Is_Working=True
             if (os.stat(outputfilename)[6]==0):
                 process.kill()
                 return
     if process.returncode != None: 
         break

    output, errors=process.communicate()

Upvotes: 1

Views: 2509

Answers (2)

jfs
jfs

Reputation: 414795

To kill the child process in 2 minutes if outputfilename is empty (I assume outputfilename is modified by some external process), you could use threading.Timer:

import os
from subprocess import Popen, PIPE
from threading import Timer

def kill_if_empty(proc, filename):
    if proc.poll() is None and os.path.getsize(filename) == 0:
       proc.kill()

def execute(command, outputfilename):
    p = Popen(command, stdout=PIPE, stderr=PIPE)
    Timer(2*60, kill_if_empty, [p, outputfilename]).start()
    output, errors = p.communicate()
    ...

This code collects stdout/stderr separately and it avoids a possible deadlock that is present in your code in the question.

Upvotes: 0

zmo
zmo

Reputation: 24802

globally your code looks good to me. Only a few details:

  1. in (time.time()-start_time)//60>1, I think there's a useless use of //, as you do not necessarily need to floor the result, and convert to integer the result of the lhs of the division. keeping it all float should be ok for the comparision, it's all basic machine logic ;
  2. you could avoid to break from an infinite loop, by changing the loop condition with while process.returncode is not None:…
  3. to make it even simpler, I'd actually loop until the file size is !=0 and then call process.wait() just after the loop.
  4. which would then be a good scenario for using the while/else construct

So, one improvement would be, so you can do stuff (like cleanup or retry…) after the process has finished (whether it succeeded or failed):

def execute(command,outputfilename):
    process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
    start_time=time.time()
    Is_Working=False
    while process.returncode == None:
        process.poll()
        #allow 2 minutes for the process to create results
        if (time.time()-start_time)/60 > 1:
            if (os.stat(outputfilename)[6]==0):
                process.kill()
                break
    else:
        # the process has not been killed
        # wait until it finishes
        process.wait()
        output, errors=process.communicate()
        # do stuff with the output
        […]

    # here the process may have been killed or not
    […]

or the other good option would be to raise an exception:

def execute(command,outputfilename):
    process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
    start_time=time.time()
    Is_Working=False
    while process.returncode == None:
        process.poll()
        #allow 2 minutes for the process to create results
        if (time.time()-start_time)/60 > 1:
            if (os.stat(outputfilename)[6]==0):
                process.kill()
                raise Exception("Process has failed to do its job")
    # the process has not been killed
    # wait until it finishes
    process.wait()
    output, errors=process.communicate()
    # do stuff with the output
    […]

HTH

Upvotes: 1

Related Questions