Pass all flake8 tests - run in CI too

.travis.yml

@@ -2,8 +2,8 @@ language: python
 python:
   - "2.7"
 # command to install dependencies, e.g. pip install -r requirements.txt --use-mirrors
-install: pip install --use-mirrors pystache nose argparse
+install: pip install --use-mirrors pystache nose argparse flake8
 # # command to run tests, e.g. python setup.py test
-script:  nosetests
+script:  ./run_tests.sh
 notifications:
   irc: "irc.freenode.org#tripleo"

os_config_applier/config_exception.py

@@ -1,2 +1,2 @@
 class ConfigException(Exception):
-  pass
+    pass

os_config_applier/os_config_applier.py

@@ -4,101 +4,125 @@ import logging
 import os
 import pystache
 import sys
-import tempfile
 from argparse import ArgumentParser
 from pystache.context import KeyNotFoundError
 from subprocess import Popen, PIPE
-from value_types import *
-from config_exception import *
 from tempfile import NamedTemporaryFile
+from value_types import ensure_type
+from config_exception import ConfigException
 
 TEMPLATES_DIR = os.environ.get('OS_CONFIG_APPLIER_TEMPLATES',
                                '/opt/stack/os-config-applier/templates')
 
-def install_config(config_path, template_root, output_path, validate, subhash=None):
+
-  config = strip_hash( read_config(config_path), subhash)
+def install_config(config_path, template_root,
-  tree = build_tree( template_paths(template_root), config )
+                   output_path, validate, subhash=None):
-  if not validate:
+    config = strip_hash(read_config(config_path), subhash)
-    for path, contents in tree.items():
+    tree = build_tree(template_paths(template_root), config)
-      write_file( os.path.join(output_path, strip_prefix('/', path)), contents)
+    if not validate:
+        for path, contents in tree.items():
+            write_file(os.path.join(
+                output_path, strip_prefix('/', path)), contents)
+
 
 def print_key(config_path, key, type_name):
-  config = read_config(config_path)
+    config = read_config(config_path)
-  keys = key.split('.')
+    keys = key.split('.')
-  for key in keys:
+    for key in keys:
-    try:
+        try:
-      config = config[key]
+            config = config[key]
-    except KeyError:
+        except KeyError:
-      raise KeyError('key %s does not exist in %s' % (key, config_path))
+            raise KeyError('key %s does not exist in %s' % (key, config_path))
-  ensure_type(config, type_name)
+    ensure_type(config, type_name)
-  print config
+    print config
 
 
 def write_file(path, contents):
-  logger.info("writing %s", path)
+    logger.info("writing %s", path)
-  d = os.path.dirname(path)
+    d = os.path.dirname(path)
-  os.path.exists(d) or os.makedirs(d)
+    os.path.exists(d) or os.makedirs(d)
-  with NamedTemporaryFile(dir=d, delete=False) as newfile:
+    with NamedTemporaryFile(dir=d, delete=False) as newfile:
-    newfile.write(contents)
+        newfile.write(contents)
-    os.rename(newfile.name, path)
+        os.rename(newfile.name, path)
 
 # return a map of filenames->filecontents
+
+
 def build_tree(templates, config):
-  res = {}
+    res = {}
-  for in_file, out_file in templates:
+    for in_file, out_file in templates:
-    res[out_file] = render_template(in_file, config)
+        res[out_file] = render_template(in_file, config)
-  return res
+    return res
+
 
 def render_template(template, config):
-  if is_executable(template):
+    if is_executable(template):
-    return render_executable(template, config)
+        return render_executable(template, config)
-  else:
+    else:
-    try:
+        try:
-      return render_moustache(open(template).read(), config)
+            return render_moustache(open(template).read(), config)
-    except KeyNotFoundError as e:
+        except KeyNotFoundError as e:
-      raise ConfigException("key '%s' from template '%s' does not exist in metadata file." % (e.key, template))
+            raise ConfigException(
-    except Exception as e:
+                "key '%s' from template '%s' does not exist in metadata file."
-      raise ConfigException("could not render moustache template %s" % template)
+                % (e.key, template))
+        except Exception as e:
+            raise ConfigException(
+                "could not render moustache template %s" % template)
+
 
 def is_executable(path):
-  return os.path.isfile(path) and os.access(path, os.X_OK)
+    return os.path.isfile(path) and os.access(path, os.X_OK)
+
 
 def render_moustache(text, config):
-  r = pystache.Renderer(missing_tags = 'strict')
+    r = pystache.Renderer(missing_tags='strict')
-  return r.render(text, config)
+    return r.render(text, config)
+
 
 def render_executable(path, config):
-  p = Popen([path], stdin=PIPE, stdout=PIPE, stderr=PIPE)
+    p = Popen([path], stdin=PIPE, stdout=PIPE, stderr=PIPE)
-  stdout, stderr = p.communicate(json.dumps(config))
+    stdout, stderr = p.communicate(json.dumps(config))
-  p.wait()
+    p.wait()
-  if p.returncode != 0: raise ConfigException("config script failed: %s\n\nwith output:\n\n%s" % (path, stdout + stderr))
+    if p.returncode != 0:
-  return stdout
+        raise ConfigException(
+            "config script failed: %s\n\nwith output:\n\n%s" %
+            (path, stdout + stderr))
+    return stdout
+
 
 def read_config(path):
-  try:
+    try:
-    return json.loads(open(path).read())
+        return json.loads(open(path).read())
-  except:
+    except:
-    raise ConfigException("invalid metadata file: %s" % path)
+        raise ConfigException("invalid metadata file: %s" % path)
+
 
 def template_paths(root):
-  res = []
+    res = []
-  for cur_root, subdirs, files in os.walk(root):
+    for cur_root, subdirs, files in os.walk(root):
-    for f in files:
+        for f in files:
-      inout = ( os.path.join(cur_root, f), os.path.join(strip_prefix(root, cur_root), f) )
+            inout = (os.path.join(cur_root, f), os.path.join(
-      res.append(inout)
+                strip_prefix(root, cur_root), f))
-  return res
+            res.append(inout)
+    return res
+
 
 def strip_prefix(prefix, s):
-  return s[len(prefix):] if s.startswith(prefix) else s
+    return s[len(prefix):] if s.startswith(prefix) else s
+
 
 def strip_hash(h, keys):
-  if not keys: return h
+    if not keys:
-  for k in keys.split('.'):
+        return h
-    if k in h and isinstance(h[k], dict):
+    for k in keys.split('.'):
-      h = h[k]
+        if k in h and isinstance(h[k], dict):
-    else:
+            h = h[k]
-      raise ConfigException("key '%s' does not correspond to a hash in the metadata file" % keys)
+        else:
-  return h
+            raise ConfigException(
+                "key '%s' does not correspond to a hash in the metadata file"
+                % keys)
+    return h
+
 
 def parse_opts(argv):
     parser = ArgumentParser()
@@ -107,58 +131,68 @@ def parse_opts(argv):
                         %(default)s)""",
                         default=TEMPLATES_DIR)
     parser.add_argument('-o', '--output', metavar='OUT_DIR',
-                        help='root directory for output (default: %(default)s)',
+                        help='root directory for output (default:%(default)s)',
                         default='/')
     parser.add_argument('-m', '--metadata', metavar='METADATA_FILE',
                         help='path to metadata file (default: %(default)s)',
                         default='/var/lib/cloud/data/cfn-init-data')
-    parser.add_argument('-v', '--validate', help='validate only. do not write files',
+    parser.add_argument(
-                        default=False, action='store_true')
+        '-v', '--validate', help='validate only. do not write files',
-    parser.add_argument('--print-templates', default=False, action='store_true',
+        default=False, action='store_true')
-                        help='Print templates root and exit.')
+    parser.add_argument(
+        '--print-templates', default=False, action='store_true',
+        help='Print templates root and exit.')
     parser.add_argument('-s', '--subhash',
-                        help='use the sub-hash named by this key, instead of the full metadata hash')
+                        help='use the sub-hash named by this key,'
+                             ' instead of the full metadata hash')
     parser.add_argument('--key', metavar='KEY', default=None,
-                        help='print the specified key and exit. (may be used with --type)')
+                        help='print the specified key and exit.'
+                             ' (may be used with --type)')
     parser.add_argument('--type', default='default',
-                        help='exit with error if the specified --key does not match type. Valid types are <int|default|raw>')
+                        help='exit with error if the specified --key does not'
+                             ' match type. Valid types are <int|default|raw>')
     opts = parser.parse_args(argv[1:])
 
     return opts
 
+
 def main(argv=sys.argv):
-  opts = parse_opts(argv)
+    opts = parse_opts(argv)
-  if opts.print_templates:
+    if opts.print_templates:
-      print(opts.templates)
+        print(opts.templates)
-      return 0
+        return 0
 
-  try:
+    try:
-    if opts.templates is None:
+        if opts.templates is None:
-        raise ConfigException('missing option --templates')
+            raise ConfigException('missing option --templates')
 
-    if opts.key:
+        if opts.key:
-      print_key(opts.metadata, opts.key, opts.type)
+            print_key(opts.metadata, opts.key, opts.type)
-    else:
+        else:
-      if not os.access(opts.output, os.W_OK):
+            if not os.access(opts.output, os.W_OK):
-        raise ConfigException("you don't have permission to write to '%s'" % opts.output)
+                raise ConfigException(
-      install_config(opts.metadata, opts.templates, opts.output,
+                    "you don't have permission to write to '%s'" % opts.output)
-                     opts.validate, opts.subhash)
+            install_config(opts.metadata, opts.templates, opts.output,
-      logger.info("success")
+                           opts.validate, opts.subhash)
-  except ConfigException as e:
+            logger.info("success")
-    logger.error(e)
+    except ConfigException as e:
-    sys.exit(1)
+        logger.error(e)
-  sys.exit(0)
+        sys.exit(1)
+    sys.exit(0)
 
 # logginig
 LOG_FORMAT = '[%(asctime)s] [%(levelname)s] %(message)s'
 DATE_FORMAT = '%Y/%m/%d %I:%M:%S %p'
+
+
 def add_handler(logger, handler):
-  handler.setFormatter(logging.Formatter(LOG_FORMAT, datefmt=DATE_FORMAT))
+    handler.setFormatter(logging.Formatter(LOG_FORMAT, datefmt=DATE_FORMAT))
-  logger.addHandler(handler)
+    logger.addHandler(handler)
 logger = logging.getLogger('os-config-applier')
 logger.setLevel(logging.INFO)
 add_handler(logger, logging.StreamHandler(sys.stdout))
-if os.geteuid() == 0: add_handler(logger, logging.FileHandler('/var/log/os-config-applier.log'))
+if os.geteuid() == 0:
+    add_handler(logger, logging.FileHandler('/var/log/os-config-applier.log'))
 
 if __name__ == '__main__':
-  main(sys.argv)
+    main(sys.argv)

os_config_applier/value_types.py

@@ -2,14 +2,17 @@ import re
 from config_exception import ConfigException
 
 TYPES = {
-  "int": "^[0-9]+$",
+    "int": "^[0-9]+$",
-  "default": "^[A-Za-z0-9]+$",
+    "default": "^[A-Za-z0-9]+$",
-  "raw": "."
+    "raw": "."
 }
 
+
 def ensure_type(string_value, type_name='default'):
-  if type_name not in TYPES:
+    if type_name not in TYPES:
-    raise ValueError("requested validation of unknown type: %s" % type_name)
+        raise ValueError(
-  if not re.match(TYPES[type_name], string_value):
+            "requested validation of unknown type: %s" % type_name)
-    raise ConfigException("cannot interpret value '%s' as type %s" % (string_value, type_name))
+    if not re.match(TYPES[type_name], string_value):
-  return string_value
+        raise ConfigException("cannot interpret value '%s' as type %s" % (
+            string_value, type_name))
+    return string_value

run_tests.sh

@@ -0,0 +1,2 @@
+flake8 --verbose `find . -name '*.py'`
+nosetests

setup.py

@@ -16,7 +16,8 @@ config = {
     'install_requires': ['pystache', 'anyjson'],
 #    'long_description': open('README.md').read(),
     'entry_points': {
-      'console_scripts': ['os-config-applier = os_config_applier.os_config_applier:main']
+        'console_scripts': [
+            'os-config-applier = os_config_applier.os_config_applier:main']
     }
 }
 

tests/os_config_applier_tests.py

@@ -1,157 +1,186 @@
 import json
 import os
+import sys
 import subprocess
 import tempfile
 from StringIO import StringIO
-from nose.tools import *
+from nose.tools import assert_equal, assert_equals, assert_raises, raises
-from os_config_applier.config_exception import *
+from os_config_applier.config_exception import ConfigException
-from os_config_applier.os_config_applier import *
+from os_config_applier.os_config_applier import (
+    main, TEMPLATES_DIR, strip_hash, read_config, template_paths,
+    render_executable, render_template, render_moustache, install_config,
+    build_tree)
 
 # example template tree
 TEMPLATES = os.path.join(os.path.dirname(__file__), 'templates')
 TEMPLATE_PATHS = [
     "/etc/glance/script.conf",
     "/etc/keystone/keystone.conf"
-  ]
+]
 
 # config for example tree
 CONFIG = {
-  "x": "foo",
+    "x": "foo",
-  "database": {
+    "database": {
     "url": "sqlite:///blah"
-  }
+    }
 }
 
 # config for example tree - with subhash
 CONFIG_SUBHASH = {
-  "OpenStack::Config": {
+    "OpenStack::Config": {
     "x": "foo",
     "database": {
-      "url": "sqlite:///blah"
+        "url": "sqlite:///blah"
+    }
     }
-  }
 }
 
 # expected output for example tree
 OUTPUT = {
-  "/etc/glance/script.conf": "foo\n",
+    "/etc/glance/script.conf": "foo\n",
-  "/etc/keystone/keystone.conf": "[foo]\ndatabase = sqlite:///blah\n"
+    "/etc/keystone/keystone.conf": "[foo]\ndatabase = sqlite:///blah\n"
 }
 
+
 def setup():
-  pass
+    pass
+
 
 def teardown():
-  pass
+    pass
+
 
 def main_path():
-  return os.path.dirname(os.path.realpath(__file__)) + '/../os_config_applier/os_config_applier.py'
+    return (
+        os.path.dirname(os.path.realpath(__file__)) +
+        '/../os_config_applier/os_config_applier.py')
+
 
 def template(relpath):
-  return os.path.join(TEMPLATES, relpath[1:])
+    return os.path.join(TEMPLATES, relpath[1:])
+
 
 def test_install_config():
-  t = tempfile.NamedTemporaryFile()
+    t = tempfile.NamedTemporaryFile()
-  t.write(json.dumps(CONFIG))
+    t.write(json.dumps(CONFIG))
-  t.flush()
+    t.flush()
-  tmpdir = tempfile.mkdtemp()
+    tmpdir = tempfile.mkdtemp()
-  install_config(t.name, TEMPLATES, tmpdir, False)
+    install_config(t.name, TEMPLATES, tmpdir, False)
-  for path, contents in OUTPUT.items():
+    for path, contents in OUTPUT.items():
-    full_path = os.path.join(tmpdir, path[1:])
+        full_path = os.path.join(tmpdir, path[1:])
-    assert os.path.exists(full_path)
+        assert os.path.exists(full_path)
-    assert_equal( open(full_path).read(), contents )
+        assert_equal(open(full_path).read(), contents)
+
 
 def test_install_config_subhash():
-  t = tempfile.NamedTemporaryFile()
+    t = tempfile.NamedTemporaryFile()
-  t.write(json.dumps(CONFIG_SUBHASH))
+    t.write(json.dumps(CONFIG_SUBHASH))
-  t.flush()
+    t.flush()
-  tmpdir = tempfile.mkdtemp()
+    tmpdir = tempfile.mkdtemp()
-  install_config(t.name, TEMPLATES, tmpdir, False, 'OpenStack::Config')
+    install_config(t.name, TEMPLATES, tmpdir, False, 'OpenStack::Config')
-  for path, contents in OUTPUT.items():
+    for path, contents in OUTPUT.items():
-    full_path = os.path.join(tmpdir, path[1:])
+        full_path = os.path.join(tmpdir, path[1:])
-    assert os.path.exists(full_path)
+        assert os.path.exists(full_path)
-    assert_equal( open(full_path).read(), contents )
+        assert_equal(open(full_path).read(), contents)
+
 
 def test_print_key():
-  t = tempfile.NamedTemporaryFile()
+    t = tempfile.NamedTemporaryFile()
-  t.write(json.dumps(CONFIG))
+    t.write(json.dumps(CONFIG))
-  t.flush()
+    t.flush()
-  out = subprocess.check_output([main_path(), '--metadata', t.name, '--key',
+    out = subprocess.check_output([main_path(), '--metadata', t.name, '--key',
-                                 'database.url', '--type', 'raw'],
+                                   'database.url', '--type', 'raw'],
-                                stderr=subprocess.STDOUT)
+                                  stderr=subprocess.STDOUT)
-  assert_equals(CONFIG['database']['url'], out.rstrip())
+    assert_equals(CONFIG['database']['url'], out.rstrip())
+
 
 @raises(subprocess.CalledProcessError)
 def test_print_key_missing():
-  t = tempfile.NamedTemporaryFile()
+    t = tempfile.NamedTemporaryFile()
-  t.write(json.dumps(CONFIG))
+    t.write(json.dumps(CONFIG))
-  t.flush()
+    t.flush()
-  out = subprocess.check_output([main_path(), '--metadata', t.name, '--key',
+    subprocess.check_output([main_path(), '--metadata', t.name, '--key',
-                                 'does.not.exist'], stderr=subprocess.STDOUT)
+                             'does.not.exist'], stderr=subprocess.STDOUT)
+
 
 @raises(subprocess.CalledProcessError)
 def test_print_key_wrong_type():
-  t = tempfile.NamedTemporaryFile()
+    t = tempfile.NamedTemporaryFile()
-  t.write(json.dumps(CONFIG))
+    t.write(json.dumps(CONFIG))
-  t.flush()
+    t.flush()
-  out = subprocess.check_output([main_path(), '--metadata', t.name, '--key',
+    subprocess.check_output([main_path(), '--metadata', t.name, '--key',
-                                 'x', '--type', 'int'],
+                             'x', '--type', 'int'], stderr=subprocess.STDOUT)
-                                stderr=subprocess.STDOUT)
 
 
 def test_build_tree():
-  assert_equals( build_tree(template_paths(TEMPLATES), CONFIG), OUTPUT )
+    assert_equals(build_tree(template_paths(TEMPLATES), CONFIG), OUTPUT)
+
 
 def test_render_template():
-  # execute executable files, moustache non-executables
+    # execute executable files, moustache non-executables
-  assert render_template(template("/etc/glance/script.conf"), {"x": "abc"}) == "abc\n"
+    assert render_template(template(
-  assert_raises(ConfigException, render_template, template("/etc/glance/script.conf"), {})
+        "/etc/glance/script.conf"), {"x": "abc"}) == "abc\n"
+    assert_raises(ConfigException, render_template, template(
+        "/etc/glance/script.conf"), {})
+
 
 def test_render_moustache():
-  assert_equals( render_moustache("ab{{x.a}}cd", {"x": {"a": "123"}}), "ab123cd" )
+    assert_equals(render_moustache("ab{{x.a}}cd", {
+                  "x": {"a": "123"}}), "ab123cd")
+
 
 @raises(Exception)
 def test_render_moustache_bad_key():
-  render_moustache("{{badkey}}", {})
+    render_moustache("{{badkey}}", {})
+
 
 def test_render_executable():
-  params = {"x": "foo"}
+    params = {"x": "foo"}
-  assert render_executable(template("/etc/glance/script.conf"), params) == "foo\n"
+    assert render_executable(template(
+        "/etc/glance/script.conf"), params) == "foo\n"
+
 
 @raises(ConfigException)
 def test_render_executable_failure():
-  render_executable(template("/etc/glance/script.conf"), {})
+    render_executable(template("/etc/glance/script.conf"), {})
+
 
 def test_template_paths():
-  expected = map(lambda p: (template(p), p), TEMPLATE_PATHS)
+    expected = map(lambda p: (template(p), p), TEMPLATE_PATHS)
-  actual = template_paths(TEMPLATES)
+    actual = template_paths(TEMPLATES)
-  expected.sort(key=lambda tup: tup[1])
+    expected.sort(key=lambda tup: tup[1])
-  actual.sort(key=lambda tup: tup[1])
+    actual.sort(key=lambda tup: tup[1])
-  assert_equals( actual , expected)
+    assert_equals(actual, expected)
+
 
 def test_read_config():
-  with tempfile.NamedTemporaryFile() as t:
+    with tempfile.NamedTemporaryFile() as t:
-    d = {"a": {"b": ["c", "d"] } }
+        d = {"a": {"b": ["c", "d"]}}
-    t.write(json.dumps(d))
+        t.write(json.dumps(d))
-    t.flush()
+        t.flush()
-    assert_equals( read_config(t.name), d )
+        assert_equals(read_config(t.name), d)
+
 
 @raises(ConfigException)
 def test_read_config_bad_json():
-  with tempfile.NamedTemporaryFile() as t:
+    with tempfile.NamedTemporaryFile() as t:
-    t.write("{{{{")
+        t.write("{{{{")
-    t.flush()
+        t.flush()
-    read_config(t.name)
+        read_config(t.name)
+
 
 @raises(Exception)
 def test_read_config_no_file():
-  read_config("/nosuchfile")
+    read_config("/nosuchfile")
+
 
 def test_strip_hash():
-  h = {'a': {'b': {'x': 'y'} }, "c": [1, 2, 3] }
+    h = {'a': {'b': {'x': 'y'}}, "c": [1, 2, 3]}
-  assert_equals( strip_hash(h, 'a.b'), {'x': 'y'})
+    assert_equals(strip_hash(h, 'a.b'), {'x': 'y'})
-  assert_raises(ConfigException, strip_hash, h, 'a.nonexistent')
+    assert_raises(ConfigException, strip_hash, h, 'a.nonexistent')
-  assert_raises(ConfigException, strip_hash, h, 'a.c')
+    assert_raises(ConfigException, strip_hash, h, 'a.c')
+
 
 def test_print_templates():
     save_stdout = sys.stdout

tests/value_type_tests.py

@@ -1,18 +1,21 @@
-from nose.tools import *
+from nose.tools import assert_equals, raises
-from os_config_applier.os_config_applier import *
+from os_config_applier.config_exception import ConfigException
-from os_config_applier.config_exception import *
+from os_config_applier.value_types import ensure_type
-from os_config_applier.value_types import *
+
 
 @raises(ValueError)
 def test_unknown_type():
-  ensure_type("foo", "badtype")
+    ensure_type("foo", "badtype")
+
 
 def test_int():
-  assert_equals("123", ensure_type("123", "int"))
+    assert_equals("123", ensure_type("123", "int"))
+
 
 def test_defualt():
-  assert_equals("foobar", ensure_type("foobar", "default"))
+    assert_equals("foobar", ensure_type("foobar", "default"))
+
 
 @raises(ConfigException)
 def test_default_bad():
-  ensure_type("foo\nbar", "default")
+    ensure_type("foo\nbar", "default")