# Gets AWS VPC subnet info

I was hoping to get a quick review of a script I wrote for retrieving AWS VPC Subnet information. Any and all ideas welcome. I am looking specifically for logic enhancements, and optimizations to make the code more pythonic. Also, I was initially constructing a dictionary, however I had to adjust since many of the aws values have "-" hyphens. :-(

One note - I am using two spaces vs. four due to internal standards.

#!/usr/bin/env python
# encoding: utf-8

import argparse
import logging
import string
import sys

from boto import vpc, exception

from lib.aws_location import LocationData
from lib.aws_util import check_config

def get_subnets(connection, vpc_id):
"""Function that returns all subnets within a given AWS VPC.
:connection: AWS Connection.
:vpc_id: VPC Identifier.

"""
return connection.get_all_subnets(filters={'vpc_id': vpc_id})

def format_data(region, vpc_id, vpc_subnet, subnets):
"""Function that outputs region data based on template.

:region: EC2 Region.
:vpc_id: VPC Identifier.
:vpc_subnet: Subnet allocated to VPC.
:subnets: Subnets defined within VPC.

todo:: Serialization. (yaml/json)
"""
with open('vpc_subnets.tmpl') as f:

region_dict = {}
region_dict['region'] = region
region_dict['vpc_id'] = vpc_id
region_dict['vpc_subnet'] = vpc_subnet
region_dict['subnets'] = ', '.join(subnets)

print templ.safe_substitute(region_dict)

def parse_args():
parser = argparse.ArgumentParser(
description='Script for obtaining VPC Subnet information.')

help="Enable debug messages",
action="store_true")

help="Enable verbose messages",
action="store_true")

args = parser.parse_args()

if args.debug:
logging.basicConfig(level=logging.DEBUG)
elif args.verbose:
logging.basicConfig(level=logging.INFO)
else:
logging.basicConfig(level=logging.CRITICAL)

def main():
parse_args()

try:
check_config()
except EnvironmentError as e:
print("Error: Could not open config file: {}".format(e))
sys.exit(1)

loc = LocationData()

for region in loc.get_regions():
try:
connection = vpc.connect_to_region(region)
for vpc_raw in loc.get_vpcs_raw(connection):
vpc_id = str(vpc_raw).split(':')[1]
subnets = []
for subnet_id in get_subnets(connection, vpc_id):
subnets.append(subnet_id.cidr_block)
format_data(region, vpc_id, vpc_raw.cidr_block, subnets)
except exception.EC2ResponseError as e:
logging.error("EC2Error: {}".format(e))
continue

if __name__ == '__main__':
main()


I haven't used much boto, so I can just focus on some style comments:

• Use four spaces per indentation level
• Remove unused imports (boto.exception)
• Try to order imports in alphabetical order
• Move argument parsing and logging configuration code to separate functions
• Be careful of the usage of next. What you were probably looking for is continue, but you can just remove that line to move on to the next region in the loop
• --dedug and --verbose don't seem to make sense together. Have a look at ArgumentParser.add_mutually_exclusive_group.
• Use a template or, at least, a multiline string to avoid writing so many print statements.
• Awesome.. some great feedback. I have a few responses, so will update the comments in my review for others. Also, I plan to make some changes based on your feedback, so going to repost in a moment. Thank you so much for reviewing. Just one non-general notes - on the imports, they are ordered according to corelibs, 3rd party libs, then my custom libs. (pep8). That said I realize, I have one out of due import due to a rename.. :-(. Jul 30 '14 at 14:33
• If you use flake8, you'll find the flake8-import-plugin useful. Jul 30 '14 at 14:48
• Incorporated all of your suggestions - except for spacing (internal standards). Thanks again for your help. Jul 30 '14 at 17:14
• Note you're not supposed to update the question code in place because that makes the question less useful to other people in the future. More information here Jul 31 '14 at 8:08
• Oh snap. Thanks for educating me. I figured someone might find the final solution helpful, but I definitely see the point of having a reference. Going to read up on that link, thanks for the info. Jul 31 '14 at 18:31