6
\$\begingroup\$

The script should be a single python file that takes an excel file (or csv). Each row represents one switch. For each switch, there will be one or more interfaces. The objective is to output a script of commands to execute to configure each switch, as specified.

For example, Row 1 is below:

switch1    Eth1    trunk    10,20,30    Eth2    Access    10    Y

The script should see that there are two interfaces. For each trunk interface, the script should output the command “swithport mode trunk”, then “switchport trunk allow vlan …” with the list of allowed VLANs. For each access interface, it’s the same, but with the word “access” instead of “trunk”.

The sample output for the above row is as below;

switch1

hostname switch1
!
interface Eth1
 switchport mode trunk
 switchport trunk allow vlan 10,20,30
!
interface Eth2
 switchport mode access
 switchport access vlan 10

Each switch’s output should be written as a separate column in a new csv or excel file.

My submitted code as below, please review and guide me for improvements;

#!/usr/bin/python
import csv
import sys
import re
import argparse


def parseCSV(path):
    '''
    Parses a CSV file and return the rows as a list
    '''
    rows = []
    try:
        f = path
    except IOError:
        print "Cannot open specified file: %s" % path
    else:
        reader = csv.reader(f)
        for row in reader:
            rows.append(row)
        f.close()
    return rows


def outputCSV(destpath, rows):
    '''
    Process the input rows and provide switch command output.
    Output then processed to destination CSV file.
    '''
    try:
        f = destpath
    except IOError:
        print "Cannot open specified file: %s" % destpath
    else:
        writer = csv.writer(f)
    for i in range(len(rows)):
        new_list = []
        final_string = ''
        if re.match(r'^switch\d$', rows[i][0]):
            new_list.append(rows[i][0])
            new_list.append('')
            new_list.append('hostname '+rows[i][0])
            new_list.append('!')
            if rows[i][1] is not None:
                new_list.append('interface '+rows[i][1].lower())
                new_list.append(' switchport mode '+rows[i][2])
                new_list.append(' switchport '+rows[i][2]+' allow vlan '+rows[i][3])
            new_list.append('!')
            if rows[i][4] is not None:
                new_list.append('interface '+rows[i][4].lower())
                new_list.append(' switchport mode '+rows[i][5].lower())
                new_list.append(' switchport '+rows[i][5].lower()+' vlan '+rows[i][6])
            final_string = '\n'.join(new_list)
            writer.writerow([final_string])
    f.close()


def main(args):
    '''
    Parses the command line arguments
    Arguments are:
        arg1: Input CSV file to process
        arg2: Output CSV file 

    Sample program execution: python myfile.py arg1 arg2
    '''
    parser = argparse.ArgumentParser(description="Reads CSV input and provide switch commands in CSV output file")
    parser.add_argument('path', help="CSV file to process", type=argparse.FileType('rt'))
    parser.add_argument('destpath', help="Output CSV file", type=argparse.FileType('at'))
    try:
        args = parser.parse_args()
    except IOError, msg:
        parser.error(str(msg))
        return
    rows = parseCSV(args.path)
    outputCSV(args.destpath, rows)


if __name__ == '__main__':
    args = sys.argv[1:]
    main(args)
\$\endgroup\$
3
\$\begingroup\$

On top of the good comments from @Jean-François Fabre's answer, I’d add the following:

  • The shebang is misleading at best. See PEP 394:

    in preparation for an eventual change in the default version of Python, Python 2 only scripts should either be updated to be source compatible with Python 3 or else to use python2 in the shebang line.

  • You don't need to care for IOErrors in your main as argparse will do it for you.

  • By turning your parseCSV function into a generator you will be able to handle very large CSVs with minimal memory footprint.
  • You state that "For each switch, there will be one or more interfaces." without specifying how many, but your code seems to assume that there is at most two, the first one always being a trunk and the second one always being an access.

    Instead, you could read the csv line by blocks of 3 elements at a time and write the configuration accordingly. The grouper recipe from itertools allow that easily.

  • You should use with statements to handle file closing.
  • You don't need to extract command line arguments using sys as argparse will do it automatically for you if you don't pass any parameter to the parse_args call.

Proposed improvements:

#!/usr/bin/env python2
import re
import csv
import argparse
from itertools import izip


def command_line_parser():
    """Create a parser for the command line arguments
    Arguments are:
        path: Input CSV file to process
        destpath: Output CSV file

    Sample program execution: python myfile.py arg1 arg2
    """
    parser = argparse.ArgumentParser(description="Reads CSV input and provide switch commands in CSV output file")
    parser.add_argument('path', help="CSV file to process", type=argparse.FileType('rt'))
    parser.add_argument('destpath', help="output CSV file", type=argparse.FileType('ab'))
    return parser


def grouper(iterable, count):
    """Collect data into fixed-length blocks"""
    args = [iter(iterable)] * count
    return izip(*args)


def parse_csv(csv_file):
    """Parses a CSV file and generate the rows one by one"""
    reader = csv.reader(csv_file)
    for line in reader:
        yield line


def write_output(output_csv, rows):
    """Process the input rows and write switch command output
    to the specified CSV file.
    """

    writer = csv.writer(output_csv)
    for row in rows:
        row_iterator = iter(row)

        switch_id = next(row_iterator)
        if not re.match(r'^switch\d$', switch_id):
            continue

        cell = ['{0}\n\nhostname {0}'.format(switch_id)]
        for interface_name, interface_type, vlans in grouper(row_iterator, 3):
            interface_type = interface_type.lower()
            allowance = 'trunk allow' if interface_type == 'trunk' else interface_type
            cell.append(
                    '!\ninterface {}\n switchport mode {}'
                    '\n switchport {} vlan {}'
                    .format(interface_name, interface_type, allowance, vlans)
            )
        writer.writerow(['\n'.join(cell)])


def main(args):
    with args.path as input_csv, args.destpath as output_csv:
        write_output(output_csv, parse_csv(input_csv))


if __name__ == '__main__':
    parser = command_line_parser()
    main(parser.parse_args())
\$\endgroup\$
  • \$\begingroup\$ "There is no need to use a csv.writer as the output file is not a CSV. Just use the write method on the opened file directly.": that's what I thought at start but it's not equivalent, as csv module adds quotes to create multi-line cells (the data written contains newlines) \$\endgroup\$ – Jean-François Fabre May 18 '18 at 13:36
  • \$\begingroup\$ @Jean-FrançoisFabre You’re right, I failed to understand that OP wanted to have each configuration on its own cell in excell. Updated \$\endgroup\$ – 409_Conflict May 18 '18 at 15:46
5
\$\begingroup\$

First, your exception wrappers are useless:

try:
    f = destpath
except IOError:
    print "Cannot open specified file: %s" % destpath

destpath is already a file object. If there were an opening problem, then argparse would have told you with an exception at this level.

parseCSV function can be rewritten in one line by turning the csv reader object to a list by iterating on it when building the list object:

def parseCSV(path):
    '''
    Parses a CSV file and return the rows as a list
    '''
    return list(csv.reader(path))

(I suggest that you close the file outside the parseCSV method as it wasn't its responsibility to open the file in the first place)

outputCSV function:

don't use the indices here, iterate on the elements:

for i in range(len(rows)):

becomes:

for row in rows:  # "rows[i]" becomes "row"

now all those append statements can be simplified/shortened by passing lists to extend.

Also note that if rows[i][1] is not None is always True: csv module returns str objects, never None.

    new_list = []
    if re.match(r'switch\d$', rows[i][0]):  # no need for ^: it's match not search
        new_list.extend([row[0],'','hostname '+row[0],'!'])
        # skipping the "always true" condition, you may need to adapt this
        new_list.extend(['interface '+row[1].lower(),' switchport mode '+row[2],' switchport '+row[2]+' allow vlan '+row[3],'!'])
        # skipping the "always true" condition, you may need to adapt this                
        new_list.extend(['interface '+row[4].lower,' switchport mode '+row[5].lower(),' switchport '+row[5].lower()+' vlan '+rows[i][6]])
        writer.writerow(['\n'.join(new_list)])

Last but not least, when using csv & text mode in Windows you stumble on the

infamous blank-line-every-two-line bug ¹ ² ³

So for python 2 (seems to be the case), you may want to change the argparse for output to:

parser.add_argument('destpath', help="Output CSV file", type=argparse.FileType('ab'))  # open as binary

and for python 3:

parser.add_argument('destpath', help="Output CSV file", type=argparse.FileType('at',newline="")) # don't issue extra CR chars, well, some magic like that :)
\$\endgroup\$
  • \$\begingroup\$ Thanks, for the review. It will help me thinking in the right direction. \$\endgroup\$ – Chandra Sekhar Samal May 18 '18 at 17:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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