Version 1.2.1

* Now PEP8 compliant - passes `flake8` code analysis
* Use `colorlog` instead of `coloredlogs` for CLI log colorization.
* Very preliminary support for OVF 2.0 format
* Greatly improved unit test coverage, including logging tests.
* Now uses `tox` and `coverage` for enhanced unit testing.
* Integration with [Coveralls](https://coveralls.io/r/glennmatthews/cot)

.coveragerc

@@ -0,0 +1,13 @@
+# .coveragerc to control coverage.py
+[run]
+omit =
+    COT/tests/*
+    COT/_version.py
+    setup.py
+    versioneer.py
+    .tox/*
+    .eggs/*
+    *easy_install*
+    */pypy/*
+    */lib_pypy/*
+    */distutils/*

.gitignore

@@ -1,2 +1,58 @@
-# Compiled python modules
-*.pyc
+# Byte-compiled / optimized / DLL files
+__pycache__/
+*.py[cod]
+
+# C extensions
+*.so
+
+# Distribution / packaging
+.Python
+env/
+build/
+develop-eggs/
+dist/
+downloads/
+eggs/
+lib/
+lib64/
+parts/
+sdist/
+var/
+*.egg-info/
+.installed.cfg
+*.egg
+
+# distutils/setuptools
+MANIFEST
+.eggs/
+
+# PyInstaller
+#  Usually these files are written by a python script from a template
+#  before PyInstaller builds the exe, so as to inject date/other infos into it.
+*.manifest
+*.spec
+
+# Installer logs
+pip-log.txt
+pip-delete-this-directory.txt
+
+# Unit test / coverage reports
+htmlcov/
+.tox/
+.coverage
+.cache
+nosetests.xml
+coverage.xml
+
+# Translations
+*.mo
+*.pot
+
+# Django stuff:
+*.log
+
+# Sphinx documentation
+docs/_build/
+
+# PyBuilder
+target/

.travis.yml

@@ -1,12 +1,14 @@
 language: python
 python:
-  - "2.6"
   - "2.7"
-  - "3.2"
-  - "3.3"
-  - "3.4"
+
 install:
-  - "pip install -r requirements.txt"
-  - "sudo python setup.py install_helpers --force"
-  - "python setup.py develop"
-script: "python setup.py test"
+  - pip install -r requirements.txt
+  - pip install tox
+  - pip install coveralls
+  - sudo python setup.py install_helpers --force
+
+script: tox
+
+after_success:
+  - coveralls

COT/__init__.py

@@ -10,8 +10,6 @@
 # of COT, including this file, may be copied, modified, propagated, or
 # distributed except according to the terms contained in the LICENSE.txt file.
 
-import os.path
-
 from ._version import get_versions
 __version__ = get_versions()['version']
 del get_versions

COT/add_disk.py

@@ -18,13 +18,13 @@
 import logging
 import os.path
 
-from .data_validation import InvalidInputError
-from .data_validation import *
-
+from .data_validation import InvalidInputError, ValueUnsupportedError
+from .data_validation import check_for_conflict, device_address, match_or_die
 from .submodule import COTSubmodule
 
 logger = logging.getLogger(__name__)
 
+
 class COTAddDisk(COTSubmodule):
     """Add or replace a disk in a virtual machine"""
 
@@ -64,7 +64,6 @@ def validate_arg(self, arg, value):
 
         return valid, value_or_reason
 
-
     def validate_controller_address(self, controller, address):
         if controller is not None:
             input_value = controller
@@ -85,7 +84,6 @@ def validate_controller_address(self, controller, address):
 
         return True, input_value
 
-
     def ready_to_run(self):
         """Are we ready to go?
         Returns the tuple (ready, reason)"""
@@ -97,15 +95,13 @@ def ready_to_run(self):
                                "specify the controller type")
         return super(COTAddDisk, self).ready_to_run()
 
-
     def run(self):
         super(COTAddDisk, self).run()
 
         add_disk_worker(self.vm,
                         UI=self.UI,
                         **self.args)
 
-
     def create_subparser(self, parent):
         p = parent.add_parser(
             'add-disk', add_help=False,
@@ -130,8 +126,8 @@ def create_subparser(self, parent):
         group.add_argument('-h', '--help', action='help',
                            help="""Show this help message and exit""")
         group.add_argument('-o', '--output',
-                           help="""Name/path of new OVF/OVA package to create """
-                           """instead of updating the existing OVF""")
+                           help="""Name/path of new OVF/OVA package to """
+                           """create instead of updating the existing OVF""")
 
         group = p.add_argument_group("disk-related options")
 
@@ -148,24 +144,24 @@ def create_subparser(self, parent):
 
         group.add_argument('-c', '--controller',
                            choices=['ide', 'scsi'],
-                           help="""Disk controller type (default: determined by """
-                           """disk type and platform)""")
+                           help="""Disk controller type (default: """
+                           """determined by disk type and platform)""")
         group.add_argument('-a', '--address', type=device_address,
-                           help="""Address of the disk, such as "1:0". Requires """
-                           """that --controller be explicitly set. """
-                            """(default: use first unused address on the """
-                            """controller)""")
+                           help="""Address of the disk, such as "1:0". """
+                           """Requires that --controller be explicitly set. """
+                           """(default: use first unused address on the """
+                           """controller)""")
         group.add_argument('-s', '--subtype',
-                           help="""Disk controller subtype such as "virtio" or """
-                           """"lsilogic".""")
+                           help="""Disk controller subtype such as "virtio" """
+                           """or "lsilogic".""")
 
         group = p.add_argument_group("descriptive options")
 
         group.add_argument('-d', '--description',
                            help="""Description of this disk (optional)""")
         group.add_argument('-n', '--name', dest='diskname',
-                           help="""Name of this disk (default: "Hard disk #" or """
-                           """"CD-ROM #" as appropriate)""")
+                           help="""Name of this disk (default: """
+                           """"Hard disk #" or "CD-ROM #" as appropriate)""")
 
         p.add_argument('DISK_IMAGE',
                        help="""Disk image file to add to the package""")
@@ -208,8 +204,8 @@ def add_disk_worker(vm,
                     "Please specify '--type harddisk' or '--type cdrom'."
                     .format(DISK_IMAGE, disk_extension,
                             ext_type_map.keys()))
-            logger.warning("New disk type not specified, guessing it should be "
-                           "'{0}' based on file extension".format(type))
+            logger.warning("New disk type not specified, guessing it should "
+                           "be '{0}' based on file extension".format(type))
 
         # Convert the disk to a new format if needed...
         DISK_IMAGE = vm.convert_disk_if_needed(DISK_IMAGE, type)
@@ -224,7 +220,7 @@ def add_disk_worker(vm,
         # Item (defines the disk drive, links to controller and File or Disk)
         #
         # For each of these four sections, we need to know whether to add
-        # a new one or overwrite an existing one. Depending on the user-provided
+        # a new one or overwrite an existing one. Depending on the user
         # arguments, we can do this by as many as three different approaches:
         #
         # 1) Check whether the DISK_IMAGE file name matches an existing File
@@ -244,7 +240,12 @@ def add_disk_worker(vm,
 
         # 2) Check whether the --file-id matches an existing File and/or Disk
         #    in the OVF (and from there, find the associated Items)
-        (f2, d2, ci2, di2) = vm.search_from_file_id(file_id)
+        # In the case where no file_id is specified, we may default to the
+        # filename, so check that instead
+        if file_id is not None:
+            (f2, d2, ci2, di2) = vm.search_from_file_id(file_id)
+        else:
+            (f2, d2, ci2, di2) = vm.search_from_file_id(disk_file)
 
         # 3) Check whether the --controller and --address match existing Items
         #    in the OVF (and from there, find the associated Disk and/or File)
@@ -277,17 +278,12 @@ def add_disk_worker(vm,
             if file is None:
                 # This will happen if we're replacing a placeholder entry
                 # (disk exists but has no associated file)
-                logger.verbose("Found Disk but not File - maybe a placeholder?")
+                logger.verbose("Found Disk but not File - maybe placeholder?")
 
         if disk_item is not None:
-            if type is not None:
-                match_or_die("disk Item ResourceType",
-                             vm.get_type_from_device(disk_item),
-                             "--type", type)
-            else:
-                type = vm.get_type_from_device(disk_item)
-                logger.info("Guessing disk type '{0}' from existing disk Item"
-                            .format(type))
+            match_or_die("disk Item ResourceType",
+                         vm.get_type_from_device(disk_item),
+                         "--type", type)
             vm.check_sanity_of_disk_device(disk, file, disk_item, ctrl_item)
 
         if ctrl_item is not None:
@@ -334,8 +330,9 @@ def add_disk_worker(vm,
             logger.warning("Overwriting existing File in OVF")
 
         if file is None and (disk is not None or disk_item is not None):
-            UI.confirm_or_die("Add disk file to existing (but empty) {0} drive?"
-                              .format(type))
+            UI.confirm_or_die(
+                "Add disk file to existing (but empty) {0} drive?"
+                .format(type))
 
         if disk is not None:
             logger.warning("Overwriting existing Disk in OVF")
@@ -382,8 +379,9 @@ def add_disk_worker(vm,
             ctrl_addr = address.split(":")[0]
             disk_addr = address.split(":")[1]
         else:
-            ctrl_addr = None # let VM choose it if necessary
-            disk_addr = 0
+            # let VM choose controller address if necessary
+            ctrl_addr = None
+            disk_addr = None
 
         ctrl_item = vm.add_controller_device(controller, subtype,
                                              ctrl_addr, ctrl_item)

COT/add_file.py

@@ -18,11 +18,12 @@
 import logging
 import sys
 
-from .data_validation import check_for_conflict, InvalidInputError
+from .data_validation import check_for_conflict
 from .submodule import COTSubmodule
 
 logger = logging.getLogger(__name__)
 
+
 class COTAddFile(COTSubmodule):
     """Add a file (such as a README) to the package."""
 
@@ -36,7 +37,6 @@ def __init__(self, UI):
                 "file_id"
             ])
 
-
     def validate_arg(self, arg, value):
         """Check whether it's OK to set the given argument to the given value.
         Returns either (True, massaged_value) or (False, reason)"""
@@ -53,19 +53,16 @@ def validate_arg(self, arg, value):
 
         return valid, value_or_reason
 
-
     def set_value(self, arg, value):
         super(COTAddFile, self).set_value(arg, value)
 
-
     def ready_to_run(self):
         """Are we ready to go?
         Returns the tuple (ready, reason)"""
         if self.get_value("FILE") is None:
             return False, "FILE is a mandatory argument!"
         return super(COTAddFile, self).ready_to_run()
 
-
     def run(self):
         super(COTAddFile, self).run()
 
@@ -88,11 +85,10 @@ def run(self):
             self.UI.confirm_or_die("Replace existing file {0} with {1}?"
                                    .format(vm.get_path_from_file(file),
                                            FILE))
-            logger.warning("Overwriting existing File in VM")
+            logger.warning("Overwriting existing File in OVF")
 
         vm.add_file(FILE, file_id, file)
 
-
     def create_subparser(self, parent):
         p = parent.add_parser(
             'add-file',