# HG changeset patch
# User cmlenz
# Date 1124735152 0
# Node ID 2efdc69e63c363d4967a82a8ba9cdbda2dd60c27
# Parent  0176b0546563cffd520c4e0ea1c49ffeb3a09b4a
Some style/documentation improvements to make Pylint happier.

diff --git a/bitten/build/shtools.py b/bitten/build/shtools.py
--- a/bitten/build/shtools.py
+++ b/bitten/build/shtools.py
@@ -96,7 +96,7 @@
         output_file = file(output, 'w')
 
     try:
-        cmdline = Commandline(executable, args, input=input_file,
+        cmdline = Commandline(executable, args, stdin=input_file,
                               cwd=ctxt.basedir)
         log_elem = xmlio.Fragment()
         for out, err in cmdline.execute():
diff --git a/bitten/master.py b/bitten/master.py
--- a/bitten/master.py
+++ b/bitten/master.py
@@ -127,8 +127,8 @@
             build.started = 0
             for step in BuildStep.select(self.env, build=build.id):
                 step.delete(db=db)
-            for log in BuildLog.select(self.env, build=build.id):
-                log.delete(db=db)
+            for build_log in BuildLog.select(self.env, build=build.id):
+                build_log.delete(db=db)
             build.update(db=db)
         db.commit()
 
@@ -162,12 +162,12 @@
                 if not platform.id in self.slaves:
                     self.slaves[platform.id] = set()
                 match = True
-                for property, pattern in ifilter(None, platform.rules):
+                for propname, pattern in ifilter(None, platform.rules):
                     try:
-                        if not re.match(pattern, handler.info.get(property)):
+                        if not re.match(pattern, handler.info.get(propname)):
                             match = False
                             break
-                    except re.error, e:
+                    except re.error:
                         log.error('Invalid platform matching pattern "%s"',
                                   pattern, exc_info=True)
                         match = False
@@ -318,13 +318,13 @@
                 db = self.env.get_db_cnx()
                 elem = xmlio.parse(payload.body)
                 if elem.name == 'started':
-                    self._build_started(db, build, elem, timestamp_delta)
+                    self._build_started(build, elem, timestamp_delta)
                 elif elem.name == 'step':
                     self._build_step_completed(db, build, elem, timestamp_delta)
                 elif elem.name == 'completed':
-                    self._build_completed(db, build, elem, timestamp_delta)
+                    self._build_completed(build, elem, timestamp_delta)
                 elif elem.name == 'aborted':
-                    self._build_aborted(db, build, elem, timestamp_delta)
+                    self._build_aborted(db, build)
                 elif elem.name == 'error':
                     build.status = Build.FAILURE
                 build.update(db=db)
@@ -343,7 +343,7 @@
                                content_encoding=encoding)
         self.channel.send_msg(message, handle_reply=handle_reply)
 
-    def _build_started(self, db, build, elem, timestamp_delta=None):
+    def _build_started(self, build, elem, timestamp_delta=None):
         build.slave = self.name
         build.slave_info.update(self.info)
         build.started = int(_parse_iso_datetime(elem.attr['time']))
@@ -370,8 +370,6 @@
             step.status = BuildStep.SUCCESS
         step.insert(db=db)
 
-        level_map = {'debug': BuildLog.DEBUG, 'info': BuildLog.INFO,
-                     'warning': BuildLog.WARNING, 'error': BuildLog.ERROR}
         for log_elem in elem.children('log'):
             build_log = BuildLog(self.env, build=build.id, step=step.name,
                                  type=log_elem.attr.get('type'))
@@ -384,7 +382,7 @@
         for report in elem.children('report'):
             store.store_report(build, step, report)
 
-    def _build_completed(self, db, build, elem, timestamp_delta=None):
+    def _build_completed(self, build, elem, timestamp_delta=None):
         log.info('Slave %s completed build %d ("%s" as of [%s]) with status %s',
                  self.name, build.id, build.config, build.rev,
                  elem.attr['result'])
@@ -396,7 +394,7 @@
         else:
             build.status = Build.SUCCESS
 
-    def _build_aborted(self, db, build, elem, timestamp_delta=None):
+    def _build_aborted(self, db, build):
         log.info('Slave "%s" aborted build %d ("%s" as of [%s])',
                  self.name, build.id, build.config, build.rev)
         build.slave = None
@@ -457,8 +455,8 @@
     env_path = args[0]
 
     # Configure logging
-    log = logging.getLogger('bitten')
-    log.setLevel(options.loglevel)
+    logger = logging.getLogger('bitten')
+    logger.setLevel(options.loglevel)
     handler = logging.StreamHandler()
     if options.logfile:
         handler.setLevel(logging.WARNING)
@@ -466,14 +464,14 @@
         handler.setLevel(options.loglevel)
     formatter = logging.Formatter('%(message)s')
     handler.setFormatter(formatter)
-    log.addHandler(handler)
+    logger.addHandler(handler)
     if options.logfile:
         handler = logging.FileHandler(options.logfile)
         handler.setLevel(options.loglevel)
         formatter = logging.Formatter('%(asctime)s [%(name)s] %(levelname)s: '
                                       '%(message)s')
         handler.setFormatter(formatter)
-        log.addHandler(handler)
+        logger.addHandler(handler)
 
     port = options.port
     if not (1 <= port <= 65535):
diff --git a/bitten/model.py b/bitten/model.py
--- a/bitten/model.py
+++ b/bitten/model.py
@@ -34,6 +34,11 @@
 
     def __init__(self, env, name=None, path=None, active=False, recipe=None,
                  min_rev=None, max_rev=None, label=None, description=None):
+        """Initialize a new build configuration with the specified attributes.
+
+        To actually create this configuration in the database, the `insert`
+        method needs to be called.
+        """
         self.env = env
         self._old_name = None
         self.name = name
@@ -48,6 +53,7 @@
     exists = property(fget=lambda self: self._old_name is not None)
 
     def insert(self, db=None):
+        """Insert a new configuration into the database."""
         assert not self.exists, 'Cannot insert existing configuration'
         assert self.name, 'Configuration requires a name'
         if not db:
@@ -69,6 +75,7 @@
         self._old_name = self.name
 
     def update(self, db=None):
+        """Save changes to an existing build configuration."""
         assert self.exists, 'Cannot update a non-existing configuration'
         assert self.name, 'Configuration requires a name'
         if not db:
@@ -90,6 +97,9 @@
         self._old_name = self.name
 
     def fetch(cls, env, name, db=None):
+        """Retrieve an existing build configuration from the database by
+        name.
+        """
         if not db:
             db = env.get_db_cnx()
 
@@ -114,6 +124,9 @@
     fetch = classmethod(fetch)
 
     def select(cls, env, include_inactive=False, db=None):
+        """Retrieve existing build configurations from the database that match
+        the specified criteria.
+        """
         if not db:
             db = env.get_db_cnx()
 
@@ -151,7 +164,12 @@
         ]
     ]
 
-    def __init__(self, env, id=None, config=None, name=None, db=None):
+    def __init__(self, env, id=None, config=None, name=None):
+        """Initialize a new target platform with the specified attributes.
+
+        To actually create this platform in the database, the `insert` method
+        needs to be called.
+        """
         self.env = env
         self.id = id
         self.config = config
@@ -161,6 +179,7 @@
     exists = property(fget=lambda self: self.id is not None)
 
     def delete(self, db=None):
+        """Remove the target platform from the database."""
         if not db:
             db = self.env.get_db_cnx()
             handle_ta = True
@@ -174,6 +193,7 @@
             db.commit()
 
     def insert(self, db=None):
+        """Insert a new target platform into the database."""
         if not db:
             db = self.env.get_db_cnx()
             handle_ta = True
@@ -199,6 +219,7 @@
             db.commit()
 
     def update(self, db=None):
+        """Save changes to an existing target platform."""
         assert self.exists, 'Cannot update a non-existing platform'
         assert self.config, 'Target platform needs to be associated with a ' \
                             'configuration'
@@ -223,6 +244,7 @@
             db.commit()
 
     def fetch(cls, env, id, db=None):
+        """Retrieve an existing target platform from the database by ID."""
         if not db:
             db = env.get_db_cnx()
 
@@ -243,6 +265,9 @@
     fetch = classmethod(fetch)
 
     def select(cls, env, config=None, db=None):
+        """Retrieve existing target platforms from the database that match the
+        specified criteria.
+        """
         if not db:
             db = env.get_db_cnx()
 
@@ -297,6 +322,11 @@
     def __init__(self, env, id=None, config=None, rev=None, platform=None,
                  slave=None, started=0, stopped=0, rev_time=0,
                  status=PENDING):
+        """Initialize a new build with the specified attributes.
+
+        To actually create this build in the database, the `insert` method needs
+        to be called.
+        """
         self.env = env
         self.slave_info = {}
         self.id = id
@@ -314,6 +344,7 @@
     successful = property(fget=lambda self: self.status == Build.SUCCESS)
 
     def delete(self, db=None):
+        """Remove the build from the database."""
         if not db:
             db = self.env.get_db_cnx()
             handle_ta = True
@@ -333,6 +364,7 @@
             db.commit()
 
     def insert(self, db=None):
+        """Insert a new build into the database."""
         if not db:
             db = self.env.get_db_cnx()
             handle_ta = True
@@ -362,6 +394,7 @@
             db.commit()
 
     def update(self, db=None):
+        """Save changes to an existing build."""
         if not db:
             db = self.env.get_db_cnx()
             handle_ta = True
@@ -388,6 +421,7 @@
             db.commit()
 
     def fetch(cls, env, id, db=None):
+        """Retrieve an existing build from the database by ID."""
         if not db:
             db = env.get_db_cnx()
 
@@ -412,6 +446,9 @@
 
     def select(cls, env, config=None, rev=None, platform=None, slave=None,
                status=None, db=None):
+        """Retrieve existing builds from the database that match the specified
+        criteria.
+        """
         if not db:
             db = env.get_db_cnx()
 
@@ -457,6 +494,11 @@
 
     def __init__(self, env, build=None, name=None, description=None,
                  status=None, started=None, stopped=None):
+        """Initialize a new build step with the specified attributes.
+
+        To actually create this build step in the database, the `insert` method
+        needs to be called.
+        """
         self.env = env
         self.build = build
         self.name = name
@@ -469,6 +511,7 @@
     successful = property(fget=lambda self: self.status == BuildStep.SUCCESS)
 
     def delete(self, db=None):
+        """Remove the build step from the database."""
         if not db:
             db = self.env.get_db_cnx()
             handle_ta = True
@@ -485,6 +528,7 @@
             db.commit()
 
     def insert(self, db=None):
+        """Insert a new build step into the database."""
         if not db:
             db = self.env.get_db_cnx()
             handle_ta = True
@@ -503,6 +547,8 @@
             db.commit()
 
     def fetch(cls, env, build, name, db=None):
+        """Retrieve an existing build from the database by build ID and step
+        name."""
         if not db:
             db = env.get_db_cnx()
 
@@ -519,6 +565,9 @@
     fetch = classmethod(fetch)
 
     def select(cls, env, build=None, name=None, db=None):
+        """Retrieve existing build steps from the database that match the
+        specified criteria.
+        """
         if not db:
             db = env.get_db_cnx()
 
@@ -563,6 +612,11 @@
     ERROR = 'E'
 
     def __init__(self, env, build=None, step=None, type=None):
+        """Initialize a new build log with the specified attributes.
+
+        To actually create this build log in the database, the `insert` method
+        needs to be called.
+        """
         self.env = env
         self.id = None
         self.build = build
@@ -573,6 +627,7 @@
     exists = property(fget=lambda self: self.id is not None)
 
     def delete(self, db=None):
+        """Remove the build log from the database."""
         assert self.exists, 'Cannot delete a non-existing build log'
         if not db:
             db = self.env.get_db_cnx()
@@ -590,6 +645,7 @@
         self.id = None
 
     def insert(self, db=None):
+        """Insert a new build log into the database."""
         if not db:
             db = self.env.get_db_cnx()
             handle_ta = True
@@ -612,6 +668,7 @@
         self.id = id
 
     def fetch(cls, env, id, db=None):
+        """Retrieve an existing build from the database by ID."""
         if not db:
             db = env.get_db_cnx()
 
@@ -632,6 +689,9 @@
     fetch = classmethod(fetch)
 
     def select(cls, env, build=None, step=None, type=None, db=None):
+        """Retrieve existing build logs from the database that match the
+        specified criteria.
+        """
         if not db:
             db = env.get_db_cnx()
 
diff --git a/bitten/recipe.py b/bitten/recipe.py
--- a/bitten/recipe.py
+++ b/bitten/recipe.py
@@ -21,7 +21,6 @@
 import keyword
 import logging
 import os
-import time
 
 from bitten.build import BuildError
 from bitten.util import xmlio
diff --git a/bitten/slave.py b/bitten/slave.py
--- a/bitten/slave.py
+++ b/bitten/slave.py
@@ -24,7 +24,6 @@
 import os
 import platform
 import shutil
-import sys
 import tempfile
 
 from bitten.build import BuildError
@@ -87,7 +86,7 @@
                     processor = config.get(section, 'processor', processor)
                 elif section == 'os':
                     system = config.get(section, 'name', system)
-                    family = config.get(section, 'family', family)
+                    family = config.get(section, 'family', os.name)
                     release = config.get(section, 'version', release)
                 else: # a package
                     attrs = {}
@@ -99,7 +98,7 @@
         log.info('Registering with build master as %s', node)
         xml = xmlio.Element('register', name=node)[
             xmlio.Element('platform', processor=processor)[machine],
-            xmlio.Element('os', family=os.name, version=release)[system],
+            xmlio.Element('os', family=family, version=release)[system],
             xmlio.Fragment()[packages]
         ]
         self.channel.send_msg(beep.Payload(xml), handle_reply)
@@ -255,13 +254,13 @@
     else:
         port = 7633
 
-    log = logging.getLogger('bitten')
-    log.setLevel(options.loglevel)
+    logger = logging.getLogger('bitten')
+    logger.setLevel(options.loglevel)
     handler = logging.StreamHandler()
     handler.setLevel(options.loglevel)
     formatter = logging.Formatter('[%(levelname)-8s] %(message)s')
     handler.setFormatter(formatter)
-    log.addHandler(handler)
+    logger.addHandler(handler)
 
     slave = Slave(host, port, options.name, options.config)
     try:
diff --git a/bitten/util/archive.py b/bitten/util/archive.py
--- a/bitten/util/archive.py
+++ b/bitten/util/archive.py
@@ -18,8 +18,7 @@
 #
 # Author: Christopher Lenz <cmlenz@gmx.de>
 
-import logging
-import os.path
+import os
 import tarfile
 import time
 import zipfile
@@ -29,17 +28,18 @@
 
 def index(env, prefix):
     """Generator that yields `(rev, format, path)` tuples for every archive in
-    the environment snapshots directory that match the specified prefix."""
+    the environment snapshots directory that match the specified prefix.
+    """
     filedir = os.path.join(env.path, 'snapshots')
     for filename in [f for f in os.listdir(filedir) if f.startswith(prefix)]:
         rest = filename[len(prefix):]
 
         # Determine format based of file extension
         format = None
-        for fmt, (ext, comp) in _formats.items():
-            if rest.endswith(ext):
-                rest = rest[:-len(ext)]
-                format = fmt
+        for name, (extension, _) in _formats.items():
+            if rest.endswith(extension):
+                rest = rest[:-len(extension)]
+                format = name
         if not format:
             continue
 
@@ -104,7 +104,7 @@
 def unpack(filename, dest_path, format=None):
     """Extract the contents of a snapshot archive."""
     if not format:
-        for name, (extension, compression) in _formats.items():
+        for name, (extension, _) in _formats.items():
             if filename.endswith(extension):
                 format = name
                 break
@@ -114,16 +114,17 @@
 
     names = []
     if format in ('bzip2', 'gzip'):
-        tar = tarfile.open(filename)
-        for tarinfo in tar:
+        tar_file = tarfile.open(filename)
+        for tarinfo in tar_file:
             names.append(tarinfo.name)
-            tar.extract(tarinfo, dest_path)
+            tar_file.extract(tarinfo, dest_path)
     elif format == 'zip':
-        zip = zipfile.ZipFile(filename, 'r')
-        for name in zip.namelist():
+        zip_file = zipfile.ZipFile(filename, 'r')
+        for name in zip_file.namelist():
             names.append(name)
+            path = os.path.join(dest_path, name)
             if name.endswith('/'):
-                os.makedirs(os.path.join(path, name))
+                os.makedirs(path)
             else:
-                file(os.path.join(path, name), 'wb').write(zip.read(name))
+                file(path, 'wb').write(zip_file.read(name))
     return os.path.commonprefix(names)
diff --git a/bitten/util/cmdline.py b/bitten/util/cmdline.py
--- a/bitten/util/cmdline.py
+++ b/bitten/util/cmdline.py
@@ -35,7 +35,7 @@
     """Simple helper for executing subprocesses."""
     # TODO: Use 'subprocess' module if available (Python >= 2.4)
 
-    def __init__(self, executable, args, input=None, cwd=None):
+    def __init__(self, executable, args, stdin=None, cwd=None):
         """Initialize the Commandline object.
         
         @param executable The name of the program to execute
@@ -129,12 +129,13 @@
                 except AttributeError:
                     fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.FNDELAY)
 
-            out_file, err_file = pipe.fromchild, pipe.childerr
-            map(make_non_blocking, [out_file.fileno(), err_file.fileno()])
+            out_file, err_file = [make_non_blocking(fd.fileno()) for fd
+                                  in (pipe.fromchild, pipe.childerr)]
             out_data, err_data = [], []
             out_eof = err_eof = False
             while not out_eof or not err_eof:
-                to_check = [out_file] * (not out_eof) + [err_file] * (not err_eof)
+                to_check = [out_file] * (not out_eof) + \
+                           [err_file] * (not err_eof)
                 ready = select.select(to_check, [], [], timeout)
                 if not ready[0]:
                     raise TimeoutError, 'Command %s timed out' % self.executable
diff --git a/bitten/util/xmlio.py b/bitten/util/xmlio.py
--- a/bitten/util/xmlio.py
+++ b/bitten/util/xmlio.py
@@ -18,26 +18,35 @@
 #
 # Author: Christopher Lenz <cmlenz@gmx.de>
 
+"""Utility code for easy input and output of XML.
+
+The current implementation uses `xml.dom.minidom` under the hood for parsing.
+"""
+
 import os
 try:
     from cStringIO import StringIO
 except ImportError:
     from StringIO import StringIO
 
-
 __all__ = ['Element', 'parse']
 
 def _escape_text(text):
+    """Escape special characters in the provided text so that it can be safely
+    included in XML text nodes.
+    """
     return str(text).replace('&', '&amp;').replace('<', '&lt;') \
                     .replace('>', '&gt;')
 
 def _escape_attr(attr):
+    """Escape special characters in the provided text so that it can be safely
+    included in XML attribute values.
+    """
     return _escape_text(attr).replace('"', '&#34;')
 
 
 class Fragment(object):
     """A collection of XML elements."""
-    
     __slots__ = ['children']
 
     def __init__(self):
@@ -161,16 +170,17 @@
 
 
 class SubElement(Element):
-
+    """Element that is appended as a new child to another element on
+    construction.
+    """
     __slots__ = []
 
     def __init__(self, parent_, name_, **attr):
         """Create an XML element using the specified tag name.
         
-        The first positional argument is the instance of the parent element that
-        this subelement should be appended to; the second positional argument is
-        the name of the tag. All keyword arguments are handled as attributes of
-        the element.
+        The first argument is the instance of the parent element that this
+        subelement should be appended to; the second argument is the name of the
+        tag. All keyword arguments are added as attributes of the element.
         """
         Element.__init__(self, name_, **attr)
         parent_.append(self)
@@ -180,20 +190,28 @@
     """Exception thrown when there's an error parsing an XML document."""
 
 
-def parse(text):
+def parse(text_or_file):
+    """Parse an XML document provided as string or file-like object.
+    
+    Returns an instance of `ParsedElement` that can be used to traverse the
+    parsed document.
+    """
     from xml.dom import minidom
     from xml.parsers import expat
     try:
-        if isinstance(text, (str, unicode)):
-            dom = minidom.parseString(text)
+        if isinstance(text_or_file, (str, unicode)):
+            dom = minidom.parseString(text_or_file)
         else:
-            dom = minidom.parse(text)
+            dom = minidom.parse(text_or_file)
         return ParsedElement(dom.documentElement)
     except expat.error, e:
         raise ParseError, e
 
 
 class ParsedElement(object):
+    """Representation of an XML element that was parsed from a string or
+    file.
+    """
     __slots__ = ['_node', 'attr']
 
     def __init__(self, node):
@@ -205,6 +223,11 @@
     namespace = property(fget=lambda self: self._node.namespaceURI)
 
     def children(self, name=None):
+        """Iterate over the child elements of this element.
+
+        If the parameter `name` is provided, only include elements with a
+        matching local name. Otherwise, include all elements.
+        """
         for child in [c for c in self._node.childNodes if c.nodeType == 1]:
             if name in (None, child.tagName):
                 yield ParsedElement(child)
@@ -213,6 +236,11 @@
         return self.children()
 
     def gettext(self):
+        """Return the text content of this element.
+        
+        This concatenates the values of all text nodes that are immediate
+        children of this element.
+        """
         return ''.join([c.nodeValue or '' for c in self._node.childNodes])
 
     def write(self, out, newlines=False):