subprocess.Popen is not running shell command

Question

I am trying to use subprocess.Popen to run 'cat test.txt | grep txt', but it's not working. In my code I have executed the subprocess.Popen command twice.

1: First time I used it to run a tshark command which redirectes the command output to a text (test.txt) file (which works fine). (defined in function get_all_tshark_out in below code)

2: Second time used subprocess.Popen to run 'cat test.txt | grep txt' command to extract txt from this file to perform some validation. This didn't work for me. (defined in function get_uniq_sessions in below code)

To make sure it's not because of buffer overflow I am flushing stdout also, but didn't get any help. Below is my code:

import subprocess
import logging

def get_all_tshark_out(logger, tcpdump, port):
    command = """tshark -r "%s" -odiameter.tcp.ports:"%s" -R 'diameter.cmd.code == 272 and diameter.flags.request==0 and !tcp.analysis.retransmission and diameter.flags.T == 0' -Tpdml -Tfields -ediameter.Session-Id | sort > test.txt""" %(tcpdump, port)
    p_out = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
    sys.stdout.flush()
    sys.stderr.flush()
    return 1

def get_uniq_sessions(logger, id='1234', uniqlog_file='test.txt'):
    command = "cat "+ uniqlog_file +" | grep "+ id
    print command
    p_out = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
    print "PPPPP", p_out
    output = p_out.stdout.read()
    p_out.wait()
    command_out_list = (output.strip().split("\n"))
    sys.stdout.flush()
    print "%%%", output, p_out.stderr.read()
    print len(command_out_list)
    if p_out.stderr.read():
        logger.error("\"%s\" Error happened while trying to execute \"%s\"" (p_out.stderr.read().strip(), command))
        sys.exit(1)
    elif command_out_list[0] == '' and len(command_out_list) == 1:
        logger.error("No Sessions belongs to %s campaign ID please provide proper input as Campaign ID" %id)
        sys.exit(1)
    else:
        return command_out_list

How do I fix this?

Answer 1

TL;DR Both of your subprocess.Popen() calls are broken; use one of the wrapper methods in subprocess instead, and/or use Python's built-in facilities instead of external tools.

Is there a particular reason you use a useless use of cat? Just subprocess.Popen(['grep', id, uniqlog_file]) would be much simpler, and not require shell=True -- but of course, Python by itself is excellent at reading files and checking whether each line contains a string.

def get_uniq_sessions(logger, id='1234', uniqlog_file='test.txt'):
    matches = []
    with open(uniqlog_file, 'r') as handle:
        for line in handle:
            if id in line:
                matches.append(line)
    return matches

Your functions should probably not call sys.exit(); instead, raise an exception, or just return None -- that way, the calling code can decide how to handle errors and exceptions.

Your remaining subprocess.Popen() only coincidentally works as long as there is a limited amount of output. You should probably use subprocess.call instead, which exists precisely for the purpose of running a subprocess under controlled conditions while checking for errors.

The key observation here is that Popen() itself merely spawns the subprocess. You need to interact with it and wait() for it in order to make sure it succeeds and returns all its output. The call and various check_* methods in the subprocess module do this for you; Popen() is useful mainly when you outgrow the facilities of those canned wrappers, but also somewhat more tricky to get right, especially the first time.

The tshark command would not require a shell=True if you picked it apart into a list yourself, and did the sorting and writing to a file in Python. If the sole purpose of the output file is to open it again from Python, I would recommend reading the raw output into a Python string and doing all the remaining processing in Python.

def get_all_tshark_out(logger, tcpdump, port):
    output = subprocess.check_output(['tshark', '-r', str(tcpdump),
        '-odiameter.tcp.ports:{0}'.format(port), '-R',
        'diameter.cmd.code == 272 and diameter.flags.request==0 '
            'and !tcp.analysis.retransmission and diameter.flags.T == 0',
        '-Tpdml', '-Tfields', '-ediameter.Session-Id'])
    return sorted(output)

... and now your get_uniq_sessions function is basically a one-liner:

 session = [x for x in get_all_tshark_out() if '1234' in x]