2
\$\begingroup\$

We have an event file which I'm processing to get the status of a job (process), whereas a single job contains multiple fields which I want to segregate based on what I required, for example, "JOB_NEW", "JOB_START" etc.

As in my current code I'm processing the file line by line and with a loop and that again processed through multiple conditions, so it works fine but looks to be time taking in case the file is really large (millions of lines). I'm wondering if there is a better nice way to process this in an elegant way to make it really better and faster.

#!/python/v3.6.1/bin/python3
import sys , os , re
from optparse import OptionParser
import numpy as np
import time
import sys
import datetime
import textwrap

wrapper = ''
preferredWidth = 75
def text_format(self,initial):
    global wrapper
    wrapper = textwrap.TextWrapper(initial_indent=initial, width=preferredWidth,
                               subsequent_indent=' '*14)
    return wrapper
wrapper = text_format('self','Job')
def opt():
        parser = OptionParser()
        parser.add_option("-l", "--longformat", dest="long",
                      help="longformat", metavar="LONG")
        parser.add_option("-n", "--number", dest="num",
                      help="number", metavar="NUM")
        (options, args) = parser.parse_args()
        return(options, args)

(options,args) = opt()

if options.long:
    jobid = options.long

check_jobid = re.match("^\d+$",jobid)
if not check_jobid:
    print(jobid + ": Illegal job ID.")
    sys.exit()

event_file = "/proj/lsb.events"

with open(event_file, "r") as r:
        for line in r:
            line = line.strip()
            match_jobid = line.split(' ')[3]
            match_jobid = match_jobid.strip()
            var = line.split()
            var = [v.replace('\"', '') for v in var]
#===============================JOB_NEW=========================================
            #if match_jobid == jobid and "JOB_NEW" in line:
            if match_jobid == jobid and var[0] == "JOB_NEW":
                print (wrapper.fill("<%s>, Job Name <%s>, User <%s>, Project <%s>, Command <%s>" %(jobid,var[-1],var[11],var[-6],var[-7])))
                wrapper = text_format('self','')
                var[2] = time.strftime('%a %b %d %H:%M:%S %Y', time.localtime(int(var[2])))
                print(wrapper.fill("%s: Submitted from host <%s>, to Queue <%s>, CWD <%s>, Output File <%s>, Requested Resources <%s>;" %(var[2],var[25],var[23],var[26],var[28],var[24])))

#==============================JOB_START========================================
            elif match_jobid == jobid and var[0] == "JOB_START":
                wrapper = text_format('self','')
                var[2] = time.strftime('%a %b %d %H:%M:%S %Y', time.localtime(int(    var[2])))
                print(wrapper.fill("%s: Dispatched to <%s>;" %(var[2],var[9])))
#=============================JOB_START_ACCEPT=================================
            elif match_jobid == jobid and var[0] == "JOB_START_ACCEPT":
                wrapper = text_format('self','')
                var[2] = time.strftime('%a %b %d %H:%M:%S %Y', \
                                time.localtime(int(var[2])))
                print(wrapper.fill("%s: Starting (Pid %s);" %(var[2],var[4])))
#=============================JOB_EXECUTE=====================================
            elif match_jobid == jobid and var[0] == "JOB_EXECUTE":
                wrapper = text_format('self','')
                var[2] = time.strftime('%a %b %d %H:%M:%S %Y', \
                                time.localtime(int(var[2])))
                print(wrapper.fill("%s: Running with execution home <%s>,\
                            Execution CWD <%s>,Execution Pid <%s> ;"\
                              %(var[2],var[7],var[6],var[9])))
#============================JOB_SIGNAL=======================================
            elif match_jobid == jobid and var[0] == "JOB_SIGNAL":
                wrapper = text_format('self','')
                var[2] = time.strftime('%a %b %d %H:%M:%S %Y', \
                                time.localtime(int(var[2])))
                print(wrapper.fill("%s: Signal <%s> requested by user \
                             or administrator <%s> ;" %(var[2],var[5],var[4])))

Sample File

"JOB_NEW" "1" 1536742813 84 258034 33554482 1 0 1536742813 0 0 "xyz" -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 "lnx64" "" "no-xyz" "/home/xyz/junk" "" "/home/xyz/123a" "cmd.err" "/home/xyz" "1536742813.84" 0 "" "" "sleep 10" "default" 2048 0 -1 "" ""
"JOB_NEW" "1" 1536742813 85 258034 33554482 1 0 1536742813 0 0 "xyz" -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 "lnx64" "" "no-xyz" "/home/xyz/junk" "" "/home/xyz/123a" "cmd.err" "/home/xyz" "1536742813.84" 0 "" "" "sleep 10" "default" 2048 0 -1 "" ""
"JOB_START" "1" 1536742819 84 4 0 0 1.0 1 "no-xyz" "" "" 0 0 ""
"JOB_START" "1" 1536742819 85 4 0 0 1.0 1 "no-xyz" "" "" 0 0 ""
"JOB_START_ACCEPT" "1" 1536742819 84 6702 6702 0 0
"JOB_EXECUTE" "1" 1536742820 84 258034 6702 "/home/xyz/junk" "/home/xyz" "xyz" 6702 0
"JOB_START_ACCEPT" "1" 1536742819 85 6702 6702 0 0
"JOB_EXECUTE" "1" 1536742820 85 258034 6702 "/home/xyz/junk" "/home/xyz" "xyz" 6702 0
"JOB_STATUS" "1" 1536742840 85 64 0 0 0.0000 1536742840 0 0 1996 0
"JOB_NEW" "1" 1536742813 86 258034 33554482 1 0 1536742813 0 0 "xyz" -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 "lnx64" "" "no-xyz" "/home/xyz/junk" "" "/home/xyz/123a" "cmd.err" "/home/xyz" "1536742813.84" 0 "" "" "sleep 10" "default" 2048 0 -1 "" ""
"JOB_START" "1" 1536742819 86 4 0 0 1.0 1 "no-xyz" "" "" 0 0 ""
"JOB_START_ACCEPT" "1" 1536742819 86 6702 6702 0 0
"JOB_EXECUTE" "1" 1536742820 86 258034 6702 "/home/xyz/junk" "/home/xyz" "xyz" 6702 0
"JOB_STATUS" "1" 1536742840 86 64 0 0 0.0000 1536742840 0 0 1996 0
"JOB_START_ACCEPT" "1" 1536742819 24 6702 6702 0 0
"JOB_EXECUTE" "1" 1536742820 14 258034 6702 "/home/xyz/junk" "/home/xyz" "xyz" 6702 0
"JOB_STATUS" "1" 1536742840 84 64 0 0 0.0000 1536742840 0 0 1996 0

Bit more details:

Here a paricular job goes through different stages (JOB_NEW , JOB_START, JOB_START_ACCEPT , JOB_EXECUTE , JOB_STATUS). Now I want to find all the lines for a paricluar job id. For example if I want to find job with job id "84", it should show the following output:

"JOB_NEW" "1" 1536742813 84 258034 33554482 1 0 1536742813 0 0 "xyz" -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 "lnx64" "" "no-xyz" "/home/xyz/junk" "" "/home/xyz/123a" "cmd.err" "/home/xyz" "1536742813.84" 0 "" "" "sleep 10" "default" 2048 0 -1 "" ""
"JOB_START" "1" 1536742819 84 4 0 0 1.0 1 "no-xyz" "" "" 0 0 ""
"JOB_START_ACCEPT" "1" 1536742819 84 6702 6702 0 0
"JOB_EXECUTE" "1" 1536742820 84 258034 6702 "/home/xyz/junk" "/home/xyz" "xyz" 6702 0

One key point is here that job id appears at column 3 (starting count from 0).

Brief context: We have a in-house tool which is used to run the Jobs or say run the batch process and records the every run with a Job ID with some JOB details as defined above, so in a nutshell it contains the complete chronology of a JOb when it started or executed which is been captured run in time in a form of event file, However the processing can be happen at given point in time by a user to see the status of his JOB with job id.

\$\endgroup\$
0

2 Answers 2

Reset to default
4
\$\begingroup\$

General practices

You have some superfluous imports (e.g. numpy) and some duplicate imports (e.g. sys). To help avoid duplication, I suggest that you put the imports in alphabetical order. Also, PEP 8 recommends against importing multiple modules on one line.

The optparse module has been deprecated in favor of argparse.

Your indentation is inconsistent, which is bad in Python, where whitespace is significant. PEP 8 says to use 4 spaces per level.

Avoid interleaving function definitions with executable statements. Function definitions should ideally go first, and the code that calls the functions should appear later, contiguously. This script is complex enough that I would avoid having any free-floating code at all, and define a main() function.

This code is especially convoluted and confusing:

wrapper = ''
preferredWidth = 75
def text_format(self,initial):
    global wrapper
    wrapper = textwrap.TextWrapper(initial_indent=initial, width=preferredWidth,
                               subsequent_indent=' '*14)
    return wrapper
wrapper = text_format('self','Job')

Why is wrapper initialized to an empty string, then reassigned twice? You reassign it once inside the text_format() function (by making wrapper global — a very bad practice!), then reassign it again from the return value of text_format(). Why does the text_format() function need a self parameter at all? As a result, you pass a silly 'self' argument every time you call the function. In fact, why do you need a text_format() function at all? You should just define a TextWrapper object just once, and use it to format all of your output.

You shouldn't have to validate jobid. You can just define the relevant parameter as required=True and type=int in the ArgumentParser.

Instead of hard-coding /proj/lsb.events as the filename, I suggest processing either a file named on the command line or sys.stdin. (If you weren't using optparse or argparse, then the fileinput module would help.)

Parsing

I don't think that you are parsing the file correctly. I see that there are three kinds of fields:

  • Integers
  • Decimals
  • Strings, which start with ", end with ", and may contain spaces (e.g. "sleep 10")

Because fields may contain spaces, splitting a line by whitespace is not a viable parsing strategy. (You certainly shouldn't split each line twice — once to get the match_jobid, and a second time for var.)

You can use a csv.reader(args.file, delimiter=' '), which supports double-quoted fields.

Formatting

The handlers for nearly all of the event types have similar code. The only caveats are:

  • For JOB_NEW events, we must first print a line which does not have a timestamp like the other lines.
  • JOB_STATUS events are ignored.
  • Each event type involves different fields.

The special requirement for JOB_NEW makes it a challenge to write a handler for the general case, but it is possible. The main trick is to use str.format() instead of performing substitutions using the % operator, which is much less powerful.

Suggested solution

import argparse
import csv
import sys
from time import localtime, strftime
from textwrap import TextWrapper

EVENT_TYPES = {
    'JOB_NEW': [
        'Job <{3}>, Job Name <{41}>, User <{11}>, Project <{36}>, Command <{35}>',
        '{time}: Submitted from host <{25}>, to Queue <{23}>, CWD <{26}>, Output File <{28}>, Requested Resources <{24}>;',
    ],
    'JOB_START': ['{time}: Dispatched to <{9}>;'],
    'JOB_START_ACCEPT': ['{time}: Starting (Pid {4});'],
    'JOB_EXECUTE': ['{time}: Running with Execution home <{7}>, Execution CWD <{6}>, Execution Pid <{9}> ;'],
    'JOB_SIGNAL': ['{time}: Signal {5} requested by user or administrator <{4}> ;'],
}

def main():
    arg_parser = argparse.ArgumentParser()
    arg_parser.add_argument('-l', '--longformat', metavar='LONG',
                            type=int, required=True, dest='long')
    arg_parser.add_argument('file', nargs='?',
                            type=argparse.FileType('r'), default=sys.stdin)
    args = arg_parser.parse_args()
    job_id = str(args.long)

    wrapper = TextWrapper(width=75, subsequent_indent=' '*14)
    for fields in csv.reader(args.file, delimiter=' '):
        if fields[3] == job_id:
            time = strftime('%a %b %d %H:%M:%S %Y', localtime(int(fields[2])))
            for fmt in EVENT_TYPES.get(fields[0], []):
                print(wrapper.fill(fmt.format(*fields, time=time)))

if __name__ == '__main__':
    main()
\$\endgroup\$
0
2
\$\begingroup\$

Programming is easier if you can split the different tasks your program needs to do into functions. I don't have time for a full review, but can give you some pointers on how to tackle this

Formatting you lines

Try to adhere to pep-8. The main thing that bothers me are too long lines, and when you split them, you do it in strange places. I found that using a code formatter (like black) helps me here. It has sane defaults, but is rather uncompromising.

global

why do you need wrapper as a global?

functions

reading the file

You can use the csv module to help you parse the lines. A simple generator which takes an open filehandle as input (hoist your IO) can be as easy as:

def read_file(input_file):
    csv_file = csv.reader(input_file, delimiter=" ", )
    for row in csv_file:
        job_info, name, job_time, job_id, *_ = row
        yield job_id, job_info, job_time, row

for each row, this uses tuple unpacking to get the info from the first few lines, and yields those

parsing

Instead of making one huge method to parse each of the different states, it is easier to make a method per different state, and use a dict to dispact each row to the correct method:

def parse_job_new(row):
    print('job new')
    return row
def parse_job_start(row):
    print('job start')
    return row
def parse_job_start_accept(row):
    print('job start accept')
    return row
def parse_job_execute(row):
    print('job execute')
    return row
def parse_job_status(row):
    print('job status')
    return row

ACTIONS = {
    "JOB_NEW": parse_job_new,
    "JOB_START": parse_job_start,
    "JOB_START_ACCEPT": parse_job_start_accept,
    "JOB_EXECUTE": parse_job_execute,
    "JOB_STATUS": parse_job_status,
}

Then you can also save the results in a collections.defaultdict

def main(input_file, job_match= None):
    results = defaultdict(dict)
    lines = read_file(input_file)
    for job_id, job_info, row in lines:
        if job_match is not None and job_id != job_match:
            continue
        results[job_id][job_info] = ACTIONS[job_info](row)
    return dict(results)

main guard

If you put the script code behind a main-guard, you can import the rest of the file in another program withtout this running all the time

if __name__ == "__main__":
    job_match = '84' # from input parsing
    with StringIO(input_str) as input_file: # with open(<filename>, 'r')
        results = main(input_file, job_match=job_match)
    print(results)

This does not do exactly as you method does, but can be a start to improve your program

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.