Merge lp:~teknico/charm-helpers/lint-fixes into lp:charm-helpers

[Nicola Larosa]

Lint and styling fixes. Sorry for the diff size.

[lp:~teknico/charm-helpers/lint-fixes]

74. By Nicola Larosa on 2013-09-02

Merge from trunk, one conflict resolved.

[James Page (james-page)]

Hi Nicola

Looking at the current master branch, this is all now fixed up.

Thanks for the MP

review: Abstain

=== modified file 'README.test'
--- README.test	2013-05-22 21:11:01 +0000
+++ README.test	2013-09-02 08:01:21 +0000
@@ -1,7 +1,10 @@
 Required Packages for Running Tests
 -----------------------------------
+
 python-shelltoolbox
 python-tempita
 python-nose
 python-mock
 python-testtools
+
+Also, install flake8 from PyPI.
 
=== modified file 'bin/contrib/saltstack/salt-call'
--- bin/contrib/saltstack/salt-call	2013-06-19 09:54:19 +0000
+++ bin/contrib/saltstack/salt-call	2013-09-02 08:01:21 +0000
@@ -1,8 +1,9 @@
 #!/usr/bin/env python
-'''
+
+"""
 Directly call a salt command in the modules, does not require a running salt
 minion to run.
-'''
+"""
 
 from salt.scripts import salt_call
 
 
=== modified file 'charmhelpers/cli/commands.py'
--- charmhelpers/cli/commands.py	2013-06-19 23:00:23 +0000
+++ charmhelpers/cli/commands.py	2013-09-02 08:01:21 +0000
@@ -1,2 +1,2 @@
-from . import CommandLine
-import host
+# from . import CommandLine
+# import host
 
=== modified file 'charmhelpers/cli/host.py'
--- charmhelpers/cli/host.py	2013-06-20 15:03:24 +0000
+++ charmhelpers/cli/host.py	2013-09-02 08:01:21 +0000
@@ -4,9 +4,10 @@
 
 @cmdline.subcommand()
 def mounts():
-    "List mounts"
+    """List mounts."""
     return host.mounts()
 
+
 @cmdline.subcommand_builder('service', description="Control system services")
 def service(subparser):
     subparser.add_argument("action", help="The action to perform (start, stop, etc...)")
 
=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2013-07-18 16:13:49 +0000
+++ charmhelpers/core/hookenv.py	2013-09-02 08:01:21 +0000
@@ -1,14 +1,15 @@
-"Interactions with the Juju environment"
 # Copyright 2013 Canonical Ltd.
 #
 # Authors:
 #  Charm Helpers Developers <[e-mail]>
 
+"""Interactions with the Juju environment."""
+
+import json
 import os
-import json
-import yaml
 import subprocess
 import UserDict
+import yaml
 
 CRITICAL = "CRITICAL"
 ERROR = "ERROR"
@@ -21,7 +22,8 @@
 
 
 def cached(func):
-    ''' Cache return values for multiple executions of func + args
+    """
+    Decorator that caches return values for multiple executions of func + args.
 
     For example:
 
@@ -32,7 +34,7 @@
         unit_get('test')
 
     will cache the result of unit_get + 'test' for future calls.
-    '''
+    """
     def wrapper(*args, **kwargs):
         global cache
         key = str((func, args, kwargs))
@@ -46,8 +48,8 @@
 
 
 def flush(key):
-    ''' Flushes any entries from function cache where the
-    key is found in the function+args '''
+    """Flush any entries from function cache where the
+    key is found in the function+args."""
     flush_list = []
     for item in cache:
         if key in item:
@@ -57,7 +59,7 @@
 
 
 def log(message, level=None):
-    "Write a message to the juju log"
+    """Write a message to the juju log."""
     command = ['juju-log']
     if level:
         command += ['-l', level]
@@ -66,7 +68,7 @@
 
 
 class Serializable(UserDict.IterableUserDict):
-    "Wrapper, an object that can be serialized to yaml or json"
+    """Wrapper, an object that can be serialized to yaml or json."""
 
     def __init__(self, obj):
         # wrap the object
@@ -96,16 +98,16 @@
         self.data = state
 
     def json(self):
-        "Serialize the object to json"
+        """Serialize the object to json."""
         return json.dumps(self.data)
 
     def yaml(self):
-        "Serialize the object to yaml"
+        """Serialize the object to yaml."""
         return yaml.dump(self.data)
 
 
 def execution_environment():
-    """A convenient bundling of the current execution context"""
+    """A convenient bundling of the current execution context."""
     context = {}
     context['conf'] = config()
     if relation_id():
@@ -119,38 +121,38 @@
 
 
 def in_relation_hook():
-    "Determine whether we're running in a relation hook"
+    """Determine whether we're running in a relation hook."""
     return 'JUJU_RELATION' in os.environ
 
 
 def relation_type():
-    "The scope for the current relation hook"
+    """The scope for the current relation hook."""
     return os.environ.get('JUJU_RELATION', None)
 
 
 def relation_id():
-    "The relation ID for the current relation hook"
+    """The relation ID for the current relation hook."""
     return os.environ.get('JUJU_RELATION_ID', None)
 
 
 def local_unit():
-    "Local unit ID"
+    """Local unit ID."""
     return os.environ['JUJU_UNIT_NAME']
 
 
 def remote_unit():
-    "The remote unit for the current relation hook"
+    """The remote unit for the current relation hook."""
     return os.environ['JUJU_REMOTE_UNIT']
 
 
 def service_name():
-    "The name service group this unit belongs to"
+    """The name service group this unit belongs to."""
     return local_unit().split('/')[0]
 
 
 @cached
 def config(scope=None):
-    "Juju charm configuration"
+    """Juju charm configuration."""
     config_cmd_line = ['config-get']
     if scope is not None:
         config_cmd_line.append(scope)
@@ -192,7 +194,7 @@
 
 @cached
 def relation_ids(reltype=None):
-    "A list of relation_ids"
+    """A list of relation_ids."""
     reltype = reltype or relation_type()
     relid_cmd_line = ['relation-ids', '--format=json']
     if reltype is not None:
@@ -203,7 +205,7 @@
 
 @cached
 def related_units(relid=None):
-    "A list of related units"
+    """A list of related units."""
     relid = relid or relation_id()
     units_cmd_line = ['relation-list', '--format=json']
     if relid is not None:
@@ -213,7 +215,7 @@
 
 @cached
 def relation_for_unit(unit=None, rid=None):
-    "Get the json represenation of a unit's relation"
+    """Get the json represenation of a unit's relation."""
     unit = unit or remote_unit()
     relation = relation_get(unit=unit, rid=rid)
     for key in relation:
@@ -225,7 +227,7 @@
 
 @cached
 def relations_for_id(relid=None):
-    "Get relations of a specific relation ID"
+    """Get relations of a specific relation ID."""
     relation_data = []
     relid = relid or relation_ids()
     for unit in related_units(relid):
@@ -237,7 +239,7 @@
 
 @cached
 def relations_of_type(reltype=None):
-    "Get relations of a specific type"
+    """Get relations of a specific type."""
     relation_data = []
     reltype = reltype or relation_type()
     for relid in relation_ids(reltype):
@@ -249,7 +251,7 @@
 
 @cached
 def relation_types():
-    "Get a list of relation types supported by this charm"
+    """Get a list of relation types supported by this charm."""
     charmdir = os.environ.get('CHARM_DIR', '')
     mdf = open(os.path.join(charmdir, 'metadata.yaml'))
     md = yaml.safe_load(mdf)
@@ -278,14 +280,14 @@
 
 
 def open_port(port, protocol="TCP"):
-    "Open a service network port"
+    """Open a service network port."""
     _args = ['open-port']
     _args.append('{}/{}'.format(port, protocol))
     subprocess.check_call(_args)
 
 
 def close_port(port, protocol="TCP"):
-    "Close a service network port"
+    """Close a service network port."""
     _args = ['close-port']
     _args.append('{}/{}'.format(port, protocol))
     subprocess.check_call(_args)
@@ -327,11 +329,10 @@
         def wrapper(decorated):
             for hook_name in hook_names:
                 self.register(hook_name, decorated)
-            else:
-                self.register(decorated.__name__, decorated)
-                if '_' in decorated.__name__:
-                    self.register(
-                        decorated.__name__.replace('_', '-'), decorated)
+            self.register(decorated.__name__, decorated)
+            if '_' in decorated.__name__:
+                self.register(
+                    decorated.__name__.replace('_', '-'), decorated)
             return decorated
         return wrapper
 
 
=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2013-08-23 16:42:43 +0000
+++ charmhelpers/core/host.py	2013-09-02 08:01:21 +0000
@@ -1,19 +1,19 @@
-"""Tools for working with the host system"""
 # Copyright 2012 Canonical Ltd.
 #
 # Authors:
 #  Nick Moffitt <[e-mail]>
 #  Matthew Wedgwood <[e-mail]>
 
+"""Tools for working with the host system."""
+
+from collections import OrderedDict
+import grp
+import hashlib
 import os
 import pwd
-import grp
 import random
 import string
 import subprocess
-import hashlib
-
-from collections import OrderedDict
 
 from hookenv import log
 
@@ -55,7 +55,7 @@
 
 
 def adduser(username, password=None, shell='/bin/bash', system_user=False):
-    """Add a user"""
+    """Add a user."""
     try:
         user_info = pwd.getpwnam(username)
         log('user {0} already exists!'.format(username))
@@ -77,7 +77,7 @@
 
 
 def add_user_to_group(username, group):
-    """Add a user to a group"""
+    """Add a user to a group."""
     cmd = [
         'gpasswd', '-a',
         username,
@@ -88,7 +88,7 @@
 
 
 def rsync(from_path, to_path, flags='-r', options=None):
-    """Replicate the contents of a path"""
+    """Replicate the contents of a path."""
     options = options or ['--delete', '--executability']
     cmd = ['/usr/bin/rsync', flags]
     cmd.extend(options)
@@ -99,7 +99,7 @@
 
 
 def symlink(source, destination):
-    """Create a symbolic link"""
+    """Create a symbolic link."""
     log("Symlinking {} as {}".format(source, destination))
     cmd = [
         'ln',
@@ -111,7 +111,7 @@
 
 
 def mkdir(path, owner='root', group='root', perms=0555, force=False):
-    """Create a directory"""
+    """Create a directory."""
     log("Making dir {} {}:{} {:o}".format(path, owner, group,
                                           perms))
     uid = pwd.getpwnam(owner).pw_uid
@@ -127,7 +127,7 @@
 
 
 def write_file(path, content, owner='root', group='root', perms=0444):
-    """Create or overwrite a file with the contents of a string"""
+    """Create or overwrite a file with the contents of a string."""
     log("Writing file {} {}:{} {:o}".format(path, owner, group, perms))
     uid = pwd.getpwnam(owner).pw_uid
     gid = grp.getgrnam(group).gr_gid
@@ -138,7 +138,7 @@
 
 
 def mount(device, mountpoint, options=None, persist=False):
-    '''Mount a filesystem'''
+    """Mount a filesystem."""
     cmd_args = ['mount']
     if options is not None:
         cmd_args.extend(['-o', options])
@@ -155,7 +155,7 @@
 
 
 def umount(mountpoint, persist=False):
-    '''Unmount a filesystem'''
+    """Unmount a filesystem."""
     cmd_args = ['umount', mountpoint]
     try:
         subprocess.check_output(cmd_args)
@@ -169,7 +169,7 @@
 
 
 def mounts():
-    '''List of all mounted volumes as [[mountpoint,device],[...]]'''
+    """List of all mounted volumes as [[mountpoint,device],[...]]."""
     with open('/proc/mounts') as f:
         # [['/mount/point','/dev/path'],[...]]
         system_mounts = [m[1::-1] for m in [l.strip().split()
@@ -178,7 +178,7 @@
 
 
 def file_hash(path):
-    ''' Generate a md5 hash of the contents of 'path' or None if not found '''
+    """Generate a md5 hash of the contents of 'path' or None if not found."""
     if os.path.exists(path):
         h = hashlib.md5()
         with open(path, 'r') as source:
@@ -189,7 +189,7 @@
 
 
 def restart_on_change(restart_map):
-    ''' Restart services based on configuration files changing
+    """Restart services based on configuration files changing.
 
     This function is used a decorator, for example
 
@@ -202,7 +202,7 @@
     In this example, the cinder-api and cinder-volume services
     would be restarted if /etc/ceph/ceph.conf is changed by the
     ceph_client_changed function.
-    '''
+    """
     def wrap(f):
         def wrapped_f(*args):
             checksums = {}
@@ -220,7 +220,7 @@
 
 
 def lsb_release():
-    '''Return /etc/lsb-release in a dict'''
+    """Return /etc/lsb-release in a dict."""
     d = {}
     with open('/etc/lsb-release', 'r') as lsb:
         for l in lsb:
@@ -230,7 +230,7 @@
 
 
 def pwgen(length=None):
-    '''Generate a random pasword.'''
+    """Generate a random password."""
     if length is None:
         length = random.choice(range(35, 45))
     alphanumeric_chars = [
 
=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2013-08-21 11:45:37 +0000
+++ charmhelpers/fetch/__init__.py	2013-09-02 08:01:21 +0000
@@ -1,18 +1,19 @@
 import importlib
-from yaml import safe_load
-from charmhelpers.core.host import (
-    lsb_release
-)
+import subprocess
 from urlparse import (
     urlparse,
     urlunparse,
 )
-import subprocess
+
+import apt_pkg
+from yaml import safe_load
+
 from charmhelpers.core.hookenv import (
     config,
     log,
 )
-import apt_pkg
+from charmhelpers.core.host import lsb_release
+
 
 CLOUD_ARCHIVE = """# Ubuntu Cloud Archive
 deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
@@ -23,7 +24,7 @@
 
 
 def filter_installed_packages(packages):
-    """Returns a list of packages that require installation"""
+    """Return a list of packages that require installation."""
     apt_pkg.init()
     cache = apt_pkg.Cache()
     _pkgs = []
@@ -39,7 +40,7 @@
 
 
 def apt_install(packages, options=None, fatal=False):
-    """Install one or more packages"""
+    """Install one or more packages."""
     options = options or []
     cmd = ['apt-get', '-y']
     cmd.extend(options)
@@ -57,7 +58,7 @@
 
 
 def apt_update(fatal=False):
-    """Update local apt cache"""
+    """Update local apt cache."""
     cmd = ['apt-get', 'update']
     if fatal:
         subprocess.check_call(cmd)
@@ -66,7 +67,7 @@
 
 
 def apt_purge(packages, fatal=False):
-    """Purge one or more packages"""
+    """Purge one or more packages."""
     cmd = ['apt-get', '-y', 'purge']
     if isinstance(packages, basestring):
         cmd.append(packages)
@@ -105,7 +106,7 @@
                       sources_var='install_sources',
                       keys_var='install_keys'):
     """
-    Configure multiple sources from charm configuration
+    Configure multiple sources from charm configuration.
 
     Example config:
         install_sources:
@@ -144,13 +145,14 @@
 
 def install_remote(source):
     """
-    Install a file tree from a remote source
+    Install a file tree from a remote source.
 
     The specified source should be a url of the form:
         scheme://[host]/path[#[option=value][&...]]
 
-    Schemes supported are based on this modules submodules
-    Options supported are submodule-specific"""
+    Schemes supported are based on this modules submodules.
+    Options supported are submodule-specific.
+    """
     # We ONLY check for True here because can_handle may return a string
     # explaining why it can't handle a given source.
     handlers = [h for h in plugins() if h.can_handle(source) is True]
@@ -172,7 +174,7 @@
 
 
 class BaseFetchHandler(object):
-    """Base class for FetchHandler implementations in fetch plugins"""
+    """Base class for FetchHandler implementations in fetch plugins."""
     def can_handle(self, source):
         """Returns True if the source can be handled. Otherwise returns
         a string explaining why it cannot"""
@@ -187,7 +189,7 @@
         return urlparse(url)
 
     def base_url(self, url):
-        """Return url without querystring or fragment"""
+        """Return url without querystring or fragment."""
         parts = list(self.parse_url(url))
         parts[4:] = ['' for i in parts[4:]]
         return urlunparse(parts)
 
=== modified file 'charmhelpers/fetch/archiveurl.py'
--- charmhelpers/fetch/archiveurl.py	2013-07-10 18:57:39 +0000
+++ charmhelpers/fetch/archiveurl.py	2013-09-02 08:01:21 +0000
@@ -1,5 +1,7 @@
 import os
 import urllib2
+
+from charmhelpers.core.host import mkdir
 from charmhelpers.fetch import (
     BaseFetchHandler,
     UnhandledSource
@@ -8,11 +10,11 @@
     get_archive_handler,
     extract,
 )
-from charmhelpers.core.host import mkdir
 
 
 class ArchiveUrlFetchHandler(BaseFetchHandler):
-    """Handler for archives via generic URLs"""
+    """Handler for archives via generic URLs."""
+
     def can_handle(self, source):
         url_parts = self.parse_url(source)
         if url_parts.scheme not in ('http', 'https', 'ftp', 'file'):
@@ -22,8 +24,7 @@
         return False
 
     def download(self, source, dest):
-        # propogate all exceptions
-        # URLError, OSError, etc
+        # Propagate all exceptions: URLError, OSError, etc.
         response = urllib2.urlopen(source)
         try:
             with open(dest, 'w') as dest_file:
 
=== modified file 'charmhelpers/fetch/bzrurl.py'
--- charmhelpers/fetch/bzrurl.py	2013-08-22 10:19:51 +0000
+++ charmhelpers/fetch/bzrurl.py	2013-09-02 08:01:21 +0000
@@ -1,10 +1,10 @@
 import os
+
+from charmhelpers.core.host import mkdir
 from charmhelpers.fetch import (
     BaseFetchHandler,
     UnhandledSource
 )
-from charmhelpers.core.host import mkdir
-
 try:
     from bzrlib.branch import Branch
 except ImportError:
@@ -12,8 +12,10 @@
     apt_install("python-bzrlib")
     from bzrlib.branch import Branch
 
+
 class BzrUrlFetchHandler(BaseFetchHandler):
-    """Handler for bazaar branches via generic and lp URLs"""
+    """Handler for bazaar branches via generic and lp URLs."""
+
     def can_handle(self, source):
         url_parts = self.parse_url(source)
         if url_parts.scheme not in ('bzr+ssh', 'lp'):
@@ -23,7 +25,7 @@
 
     def branch(self, source, dest):
         url_parts = self.parse_url(source)
-        # If we use lp:branchname scheme we need to load plugins
+        # If we use lp:branchname scheme we need to load plugins.
         if not self.can_handle(source):
             raise UnhandledSource("Cannot handle {}".format(source))
         if url_parts.scheme == "lp":
@@ -37,8 +39,9 @@
 
     def install(self, source):
         url_parts = self.parse_url(source)
-        branch_name = url_parts.path.strip("/").split("/")[-1]
-        dest_dir = os.path.join(os.environ.get('CHARM_DIR'), "fetched", branch_name)
+        branch_name = url_parts.path.strip('/').split('/')[-1]
+        dest_dir = os.path.join(
+            os.environ.get('CHARM_DIR'), 'fetched', branch_name)
         if not os.path.exists(dest_dir):
             mkdir(dest_dir, perms=0755)
         try:
@@ -46,4 +49,3 @@
         except OSError as e:
             raise UnhandledSource(e.strerror)
         return dest_dir
-
 
=== modified file 'charmhelpers/payload/__init__.py'
--- charmhelpers/payload/__init__.py	2013-05-18 23:25:48 +0000
+++ charmhelpers/payload/__init__.py	2013-09-02 08:01:21 +0000
@@ -1,1 +1,3 @@
-"Tools for working with files injected into a charm just before deployment."
+"""
+Tools for working with files injected into a charm just before deployment.
+"""
 
=== modified file 'charmhelpers/payload/archive.py'
--- charmhelpers/payload/archive.py	2013-07-03 23:04:00 +0000
+++ charmhelpers/payload/archive.py	2013-09-02 08:01:21 +0000
@@ -1,6 +1,7 @@
 import os
 import tarfile
 import zipfile
+
 from charmhelpers.core import (
     host,
     hookenv,
@@ -46,12 +47,12 @@
 
 
 def extract_tarfile(archive_name, destpath):
-    "Unpack a tar archive, optionally compressed"
+    """Unpack a tar archive, optionally compressed."""
     archive = tarfile.open(archive_name)
     archive.extractall(destpath)
 
 
 def extract_zipfile(archive_name, destpath):
-    "Unpack a zip file"
+    """Unpack a zip file."""
     archive = zipfile.ZipFile(archive_name)
     archive.extractall(destpath)
 
=== modified file 'charmhelpers/payload/execd.py'
--- charmhelpers/payload/execd.py	2013-06-07 12:34:25 +0000
+++ charmhelpers/payload/execd.py	2013-09-02 08:01:21 +0000
@@ -1,8 +1,9 @@
 #!/usr/bin/env python
 
 import os
+import subprocess
 import sys
-import subprocess
+
 from charmhelpers.core import hookenv
 
 
 
=== modified file 'tests/contrib/hahelpers/test_ceph_utils.py'
--- tests/contrib/hahelpers/test_ceph_utils.py	2013-08-13 01:12:03 +0000
+++ tests/contrib/hahelpers/test_ceph_utils.py	2013-09-02 08:01:21 +0000
@@ -126,6 +126,6 @@
         fstype = 'xfs'
         ceph_utils.make_filesystem(device, fstype)
         self.check_call.assert_called_with(['mkfs', '-t', fstype, device])
-        self.log.assert_called_with('ceph: Formatting block device %s as '
-            'filesystem %s.' % (device, fstype), level='INFO')
-
+        self.log.assert_called_with(
+            'ceph: Formatting block device %s as filesystem %s.' % (
+                device, fstype), level='INFO')
 
=== modified file 'tests/core/test_hookenv.py'
--- tests/core/test_hookenv.py	2013-08-07 17:05:39 +0000
+++ tests/core/test_hookenv.py	2013-09-02 08:01:21 +0000
@@ -1,9 +1,7 @@
+import cPickle as pickle
 import json
-
-import cPickle as pickle
-from mock import patch, call, mock_open
+from mock import call, MagicMock, mock_open, patch
 from StringIO import StringIO
-from mock import MagicMock
 from testtools import TestCase
 import yaml
 
@@ -25,6 +23,7 @@
 
 
 class SerializableTest(TestCase):
+
     def test_serializes_object_to_json(self):
         foo = {
             'bar': 'baz',
@@ -115,6 +114,7 @@
 
 
 class HelpersTest(TestCase):
+
     def setUp(self):
         super(HelpersTest, self).setUp()
         # Reset hookenv cache for each test
 
=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2013-08-23 16:42:43 +0000
+++ tests/core/test_host.py	2013-09-02 08:01:21 +0000
@@ -1,7 +1,7 @@
 from collections import OrderedDict
 from contextlib import contextmanager
+import io
 import subprocess
-import io
 
 from mock import patch, call, MagicMock
 from testtools import TestCase
@@ -18,19 +18,20 @@
                """rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0
 """).strip().split('\n')
 
-LSB_RELEASE = u'''DISTRIB_ID=Ubuntu
+LSB_RELEASE = u"""DISTRIB_ID=Ubuntu
 DISTRIB_RELEASE=13.10
 DISTRIB_CODENAME=saucy
 DISTRIB_DESCRIPTION="Ubuntu Saucy Salamander (development branch)"
-'''
+"""
 
 
 @contextmanager
 def patch_open():
-    '''Patch open() to allow mocking both open() itself and the file that is
+    """Patch open() to allow mocking both open() itself and the file that is
     yielded.
 
-    Yields the mock for "open" and "file", respectively.'''
+    Yields the mock for "open" and "file", respectively.
+    """
     mock_open = MagicMock(spec=open)
     mock_file = MagicMock(spec=file)
 
@@ -45,7 +46,7 @@
 
 @contextmanager
 def mock_open(filename, contents=None):
-    ''' Slightly simpler mock of open to return contents for filename '''
+    """Slightly simpler mock of open to return contents for filename """
     def mock_file(*args):
         if args[0] == filename:
             return io.StringIO(contents)
 
=== modified file 'tests/fetch/test_archiveurl.py'
--- tests/fetch/test_archiveurl.py	2013-07-10 18:57:39 +0000
+++ tests/fetch/test_archiveurl.py	2013-09-02 08:01:21 +0000
@@ -1,16 +1,18 @@
 import os
-from testtools import TestCase
+import urllib2
 from urlparse import urlparse
+
 from mock import (
     MagicMock,
     patch,
     mock_open,
 )
+from testtools import TestCase
+
 from charmhelpers.fetch import (
     archiveurl,
     UnhandledSource,
 )
-import urllib2
 
 
 class ArchiveUrlFetchHandlerTest(TestCase):
 
=== modified file 'tests/fetch/test_bzrurl.py'
--- tests/fetch/test_bzrurl.py	2013-08-21 11:45:37 +0000
+++ tests/fetch/test_bzrurl.py	2013-09-02 08:01:21 +0000
@@ -1,10 +1,12 @@
 import os
-from testtools import TestCase
 from urlparse import urlparse
+
 from mock import (
     MagicMock,
     patch,
 )
+from testtools import TestCase
+
 from charmhelpers.fetch import (
     bzrurl,
     UnhandledSource,
@@ -41,7 +43,6 @@
         )
         self.fh = bzrurl.BzrUrlFetchHandler()
 
-
     def test_handles_bzr_urls(self):
         for url in self.valid_urls:
             result = self.fh.can_handle(url)
@@ -50,7 +51,6 @@
             result = self.fh.can_handle(url)
             self.assertNotEqual(result, True, url)
 
-
     @patch('bzrlib.branch.Branch.open')
     def test_branch(self, _open):
         dest_path = "/destination/path"
@@ -65,7 +65,6 @@
             with patch.dict('os.environ', {'CHARM_DIR': 'foo'}):
                 self.assertRaises(UnhandledSource, self.fh.branch, url, dest_path)
 
-
     @patch('charmhelpers.fetch.bzrurl.mkdir')
     def test_installs(self, _mkdir):
         self.fh.branch = MagicMock()
@@ -77,4 +76,3 @@
                 where = self.fh.install(url)
             self.assertEqual(where, dest)
             _mkdir.assert_called_with(where, perms=0755)
-
 
=== modified file 'tests/fetch/test_fetch.py'
--- tests/fetch/test_fetch.py	2013-08-21 15:45:53 +0000
+++ tests/fetch/test_fetch.py	2013-09-02 08:01:21 +0000
@@ -1,13 +1,16 @@
 from contextlib import contextmanager
-from testtools import TestCase
+from urlparse import urlparse
+
 from mock import (
     patch,
     MagicMock,
     call,
 )
-from urlparse import urlparse
+from testtools import TestCase
+import yaml
+
 from charmhelpers import fetch
-import yaml
+
 
 FAKE_APT_CACHE = {
     # an installed package
@@ -38,10 +41,10 @@
 
 @contextmanager
 def patch_open():
-    '''Patch open() to allow mocking both open() itself and the file that is
+    """Patch open() to allow mocking both open() itself and the file that is
     yielded.
 
-    Yields the mock for "open" and "file", respectively.'''
+    Yields the mock for "open" and "file", respectively."""
     mock_open = MagicMock(spec=open)
     mock_file = MagicMock(spec=file)
 
@@ -96,9 +99,9 @@
     @patch.object(fetch, 'apt_install')
     def test_add_source_cloud(self, apt_install, filter_pkg):
         source = "cloud:havana-updates"
-        result = '''# Ubuntu Cloud Archive
+        result = """# Ubuntu Cloud Archive
 deb http://ubuntu-cloud.archive.canonical.com/ubuntu havana-updates main
-'''
+"""
         with patch_open() as (mock_open, mock_file):
             fetch.add_source(source=source)
             mock_file.write.assert_called_with(result)
@@ -352,27 +355,24 @@
         check_call.assert_called_with(['apt-get', '-y', '--foo', '--bar',
                                        'install', 'foo', 'bar'])
 
-
     @patch('subprocess.check_call')
     @patch.object(fetch, 'log')
     def test_purges_apt_packages_as_string_fatal(self, log, mock_call):
         packages = 'irrelevant names'
         mock_call.side_effect = OSError('fail')
 
-        mock_call.assertRaises(OSError, fetch.apt_purge, packages, fatal=True )
+        mock_call.assertRaises(OSError, fetch.apt_purge, packages, fatal=True)
         log.assert_called()
 
-
     @patch('subprocess.check_call')
     @patch.object(fetch, 'log')
     def test_purges_apt_packages_fatal(self, log, mock_call):
         packages = ['irrelevant', 'names']
         mock_call.side_effect = OSError('fail')
 
-        mock_call.assertRaises(OSError, fetch.apt_purge, packages, fatal=True )
+        mock_call.assertRaises(OSError, fetch.apt_purge, packages, fatal=True)
         log.assert_called()
 
-
     @patch('subprocess.call')
     @patch.object(fetch, 'log')
     def test_purges_apt_packages_as_string_nofatal(self, log, mock_call):
@@ -383,7 +383,6 @@
         log.assert_called()
         mock_call.assert_called_with(['apt-get', '-y', 'purge', 'foo bar'])
 
-
     @patch('subprocess.call')
     @patch.object(fetch, 'log')
     def test_purges_apt_packages_nofatal(self, log, mock_call):
@@ -395,7 +394,6 @@
         mock_call.assert_called_with(['apt-get', '-y', 'purge', 'foo',
                                       'bar'])
 
-
     @patch('subprocess.check_call')
     def test_apt_update_fatal(self, check_call):
         fetch.apt_update(fatal=True)
 
=== modified file 'tests/payload/test_archive.py'
--- tests/payload/test_archive.py	2013-07-03 23:04:00 +0000
+++ tests/payload/test_archive.py	2013-09-02 08:01:21 +0000
@@ -1,13 +1,15 @@
 import os
-from testtools import TestCase
+from shutil import rmtree
+import subprocess
+from tempfile import mkdtemp
+
 from mock import (
     patch,
     MagicMock,
 )
+from testtools import TestCase
+
 from charmhelpers.payload import archive
-from tempfile import mkdtemp
-from shutil import rmtree
-import subprocess
 
 
 class ArchiveTestCase(TestCase):
 
=== modified file 'tests/payload/test_execd.py'
--- tests/payload/test_execd.py	2013-07-11 08:31:49 +0000
+++ tests/payload/test_execd.py	2013-09-02 08:01:21 +0000
@@ -1,11 +1,11 @@
-from testtools import TestCase
-from mock import patch
 import os
 import shutil
 import stat
-
 from tempfile import mkdtemp
 
+from mock import patch
+from testtools import TestCase
+
 from charmhelpers.payload import execd
 
 
@@ -58,7 +58,9 @@
         expected_file = os.path.join(self.test_charm_dir, execd_dir,
                                      module_dir, 'charm-pre-install-success')
         files = os.listdir(os.path.dirname(expected_file))
-        self.assertTrue(os.path.exists(expected_file), "files were: %s. charmdir is: %s" % (files, self.test_charm_dir))
+        self.assertTrue(
+            os.path.exists(expected_file),
+            'files were: %s. charmdir is: %s' % (files, self.test_charm_dir))
 
     def test_execd_preinstall(self):
         """All charm-pre-install hooks are executed."""
 
=== modified file 'tests/tools/test_charm_helper_sync.py'
--- tests/tools/test_charm_helper_sync.py	2013-07-12 01:28:42 +0000
+++ tests/tools/test_charm_helper_sync.py	2013-09-02 08:01:21 +0000
@@ -19,7 +19,7 @@
 
 class HelperSyncTests(unittest.TestCase):
     def test_clone_helpers(self):
-        '''It properly branches the correct helpers branch'''
+        """It properly branches the correct helpers branch."""
         with patch('subprocess.check_call') as check_call:
             sync.clone_helpers(work_dir='/tmp/foo', branch='lp:charm-helpers')
             check_call.assert_called_with(['bzr', 'branch',
@@ -27,19 +27,19 @@
                                            '/tmp/foo/charm-helpers'])
 
     def test_module_path(self):
-        '''It converts a python module path to a filesystem path'''
+        """It converts a python module path to a filesystem path."""
         self.assertEquals(sync._module_path('some.test.module'),
                           'some/test/module')
 
     def test_src_path(self):
-        '''It renders the correct path to module within charm-helpers tree'''
+        """It renders the correct path to module within charm-helpers tree."""
         path = sync._src_path(src='/tmp/charm-helpers',
                               module='contrib.openstack')
         self.assertEquals('/tmp/charm-helpers/charmhelpers/contrib/openstack',
                           path)
 
     def test_dest_path(self):
-        '''It correctly finds the correct install path within a charm'''
+        """It correctly finds the correct install path within a charm."""
         path = sync._dest_path(dest='/tmp/mycharm/hooks/charmhelpers',
                                module='contrib.openstack')
         self.assertEquals('/tmp/mycharm/hooks/charmhelpers/contrib/openstack',
@@ -49,7 +49,7 @@
     @patch('os.path.exists')
     @patch('os.walk')
     def test_ensure_init(self, walk, exists, _open):
-        '''It ensures all subdirectories of a parent are python importable'''
+        """It ensures all subdirectories of a parent are python importable."""
         # os walk
         # os.path.join
         # os.path.exists
@@ -73,7 +73,7 @@
     @patch('os.makedirs')
     @patch('os.path.exists')
     def test_sync_pyfile(self, exists, mkdirs, copy, isfile, ensure_init):
-        '''It correctly syncs a py src file from src to dest'''
+        """It correctly syncs a py src file from src to dest."""
         exists.return_value = False
         isfile.return_value = True
         sync.sync_pyfile('/tmp/charm-helpers/core/host',
@@ -88,7 +88,7 @@
         ensure_init.assert_called_with('hooks/charmhelpers/core')
 
     def _test_filter_dir(self, opts, isfile, isdir):
-        '''It filters non-python files and non-module dirs from source'''
+        """It filters non-python files and non-module dirs from source."""
         files = {
             'bad_file.bin': 'f',
             'some_dir': 'd',
@@ -119,7 +119,7 @@
     @patch('os.path.isdir')
     @patch('os.path.isfile')
     def test_filter_dir_no_opts(self, isfile, isdir):
-        '''It filters out all non-py files by default'''
+        """It filters out all non-py files by default."""
         result = self._test_filter_dir(opts=None, isfile=isfile, isdir=isdir)
         ex = ['bad_file.bin', 'bad_file.img', 'some_dir']
         self.assertEquals(ex, result)
@@ -127,7 +127,7 @@
     @patch('os.path.isdir')
     @patch('os.path.isfile')
     def test_filter_dir_with_include(self, isfile, isdir):
-        '''It includes non-py files if specified as an include opt'''
+        """It includes non-py files if specified as an include opt."""
         result = self._test_filter_dir(opts=['inc=*.img'],
                                        isfile=isfile, isdir=isdir)
         ex = ['bad_file.bin', 'some_dir']
@@ -136,7 +136,7 @@
     @patch('os.path.isdir')
     @patch('os.path.isfile')
     def test_filter_dir_include_all(self, isfile, isdir):
-        '''It does not filter anything if option specified to include all'''
+        """It does not filter anything if option specified to include all."""
         self.assertEquals(sync.get_filter(opts=['inc=*']), None)
 
     @patch('tools.charm_helpers_sync.charm_helpers_sync.get_filter')
@@ -146,7 +146,7 @@
     @patch('os.path.exists')
     def test_sync_directory(self, exists, rmtree, copytree, ensure_init,
                             _filter):
-        '''It correctly syncs src directory to dest directory'''
+        """It correctly syncs src directory to dest directory."""
         _filter.return_value = None
         sync.sync_directory('/tmp/charm-helpers/charmhelpers/core',
                             'hooks/charmhelpers/core')
@@ -158,7 +158,7 @@
 
     @patch('os.path.isfile')
     def test_is_pyfile(self, isfile):
-        '''It correctly identifies incomplete path to a py src file as such'''
+        """It correctly identifies incomplete path to a py src file as such."""
         sync._is_pyfile('/tmp/charm-helpers/charmhelpers/core/host')
         isfile.assert_called_with(
             '/tmp/charm-helpers/charmhelpers/core/host.py'
@@ -167,7 +167,7 @@
     @patch('tools.charm_helpers_sync.charm_helpers_sync.sync_directory')
     @patch('os.path.isdir')
     def test_syncs_directory(self, is_dir, sync_dir):
-        '''It correctly syncs a module directory'''
+        """It correctly syncs a module directory."""
         is_dir.return_value = True
         sync.sync(src='/tmp/charm-helpers',
                   dest='hooks/charmhelpers',
@@ -181,7 +181,7 @@
     @patch('tools.charm_helpers_sync.charm_helpers_sync._is_pyfile')
     @patch('os.path.isdir')
     def test_syncs_file(self, is_dir, is_pyfile, sync_pyfile):
-        '''It correctly syncs a module file'''
+        """It correctly syncs a module file."""
         is_dir.return_value = False
         is_pyfile.return_value = True
         sync.sync(src='/tmp/charm-helpers',
@@ -195,7 +195,7 @@
     @patch('tools.charm_helpers_sync.charm_helpers_sync.sync')
     @patch('os.path.isdir')
     def test_sync_helpers_from_config(self, isdir, _sync):
-        '''It correctly syncs a list of included helpers'''
+        """It correctly syncs a list of included helpers."""
         include = yaml.load(INCLUDE)['include']
         isdir.return_value = True
         sync.sync_helpers(include=include,
@@ -219,14 +219,14 @@
         self.assertEquals(ex_calls, _sync.call_args_list)
 
     def test_extract_option_no_globals(self):
-        '''It extracts option from an included item with no global options'''
+        """It extracts option from an included item with no global options."""
         inc = 'contrib.openstack.templates|inc=*.template'
         result = sync.extract_options(inc)
         ex = ('contrib.openstack.templates', ['inc=*.template'])
         self.assertEquals(ex, result)
 
     def test_extract_option_with_global_as_string(self):
-        '''It extracts option for include with global options as str'''
+        """It extracts option for include with global options as str."""
         inc = 'contrib.openstack.templates|inc=*.template'
         result = sync.extract_options(inc, global_options='inc=foo.*')
         ex = ('contrib.openstack.templates',
@@ -234,14 +234,14 @@
         self.assertEquals(ex, result)
 
     def test_extract_option_with_globals(self):
-        '''It extracts option from an included item with global options'''
+        """It extracts option from an included item with global options."""
         inc = 'contrib.openstack.templates|inc=*.template'
         result = sync.extract_options(inc, global_options=['inc=*.cfg'])
         ex = ('contrib.openstack.templates', ['inc=*.template', 'inc=*.cfg'])
         self.assertEquals(ex, result)
 
     def test_extract_multiple_options_with_globals(self):
-        '''It extracts multiple options from an included item'''
+        """It extracts multiple options from an included item."""
         inc = 'contrib.openstack.templates|inc=*.template,inc=foo.*'
         result = sync.extract_options(inc, global_options=['inc=*.cfg'])
         ex = ('contrib.openstack.templates',
 
=== modified file 'tools/charm_helpers_sync/charm_helpers_sync.py'
--- tools/charm_helpers_sync/charm_helpers_sync.py	2013-07-12 01:28:42 +0000
+++ tools/charm_helpers_sync/charm_helpers_sync.py	2013-09-02 08:01:21 +0000
@@ -1,22 +1,19 @@
 #!/usr/bin/python
 #
 # Copyright 2013 Canonical Ltd.
-
 # Authors:
 #   Adam Gandelman <[e-mail]>
-#
 
+from fnmatch import fnmatch
 import logging
 import optparse
 import os
+import shutil
 import subprocess
-import shutil
 import sys
 import tempfile
 import yaml
 
-from fnmatch import fnmatch
-
 CHARM_HELPERS_BRANCH = 'lp:charm-helpers'
 
 
@@ -52,13 +49,13 @@
 
 
 def ensure_init(path):
-    '''
-    ensure directories leading up to path are importable, omitting
+    """
+    Ensure directories leading up to path are importable, omitting
     parent directory, eg path='/hooks/helpers/foo'/:
         hooks/
         hooks/helpers/__init__.py
         hooks/helpers/foo/__init__.py
-    '''
+    """
     for d, dirs, files in os.walk(os.path.join(*path.split('/')[:2])):
         _i = os.path.join(d, '__init__.py')
         if not os.path.exists(_i):
@@ -165,6 +162,7 @@
                         inc, opts = extract_options(m, global_options)
                         sync(src, dest, '%s.%s' % (k, inc), opts)
 
+
 if __name__ == '__main__':
     parser = optparse.OptionParser()
     parser.add_option('-c', '--config', action='store', dest='config',