Several important cleanups.

* Turn on absolute_import and unicode_literals everywhere, and deal with the
  aftermath.
* Use 'except X as Y' everywhere.
* Make the module prologues much more consistent.
* Use '{}'.format() consistently, except for logger interface.
* Because of the problems with calling ** args with unicode keywords, hide
  calls to Template.substitute() behind an API.

3 files changed, 56 insertions, 47 deletions

diff --git a/mailman/queue/__init__.py b/mailman/queue/__init__.py
index e6d39ee1b..cb2225b13 100644
--- a/mailman/queue/__init__.py
+++ b/mailman/queue/__init__.py
@@ -24,6 +24,8 @@ written.  First, the message is written to the pickle, then the metadata
 dictionary is written.
 """
 
+from __future__ import absolute_import, unicode_literals
+
 __metaclass__ = type
 __all__ = [
     'Runner',
@@ -45,7 +47,6 @@ import traceback
 
 from cStringIO import StringIO
 from lazr.config import as_boolean, as_timedelta
-from string import Template
 from zope.interface import implements
 
 from mailman import Message
@@ -54,6 +55,7 @@ from mailman import i18n
 from mailman.config import config
 from mailman.interfaces.runner import IRunner
 from mailman.interfaces.switchboard import ISwitchboard
+from mailman.utilities.string import expand
 
 # 20 bytes of all bits set, maximum hashlib.sha.digest() value
 shamax = 0xffffffffffffffffffffffffffffffffffffffffL
@@ -81,10 +83,10 @@ class Switchboard:
         for conf in config.qrunner_configs:
             name = conf.name.split('.')[-1]
             assert name not in config.switchboards, (
-                'Duplicate qrunner name: %s' % name)
+                'Duplicate qrunner name: {0}'.format(name))
             substitutions = config.paths
             substitutions['name'] = name
-            path = Template(conf.path).safe_substitute(substitutions)
+            path = expand(conf.path, substitutions)
             config.switchboards[name] = Switchboard(path)
 
     def __init__(self, queue_directory,
@@ -103,7 +105,7 @@ class Switchboard:
         :type recover: bool
         """
         assert (numslices & (numslices - 1)) == 0, (
-            'Not a power of 2: %s' % numslices)
+            'Not a power of 2: {0}'.format(numslices))
         self.queue_directory = queue_directory
         # Create the directory if it doesn't yet exist.
         Utils.makedirs(self.queue_directory, 0770)
@@ -148,7 +150,8 @@ class Switchboard:
         tmpfile = filename + '.tmp'
         # Always add the metadata schema version number
         data['version'] = config.QFILE_SCHEMA_VERSION
-        # Filter out volatile entries
+        # Filter out volatile entries.  Use .keys() so that we can mutate the
+        # dictionary during the iteration.
         for k in data.keys():
             if k.startswith('_'):
                 del data[k]
@@ -197,7 +200,7 @@ class Switchboard:
                 os.rename(bakfile, psvfile)
             else:
                 os.unlink(bakfile)
-        except EnvironmentError, e:
+        except EnvironmentError:
             elog.exception(
                 'Failed to unlink/preserve backup file: %s', bakfile)
 
@@ -246,11 +249,11 @@ class Switchboard:
                     msg = cPickle.load(fp)
                     data_pos = fp.tell()
                     data = cPickle.load(fp)
-                except Exception, s:
+                except Exception as error:
                     # If unpickling throws any exception, just log and
                     # preserve this entry
                     elog.error('Unpickling .bak exception: %s\n'
-                               'Preserving file: %s', s, filebase)
+                               'Preserving file: %s', error, filebase)
                     self.finish(filebase, preserve=True)
                 else:
                     data['_bak_count'] = data.get('_bak_count', 0) + 1
@@ -289,8 +292,7 @@ class Runner:
         section = getattr(config, 'qrunner.' + name)
         substitutions = config.paths
         substitutions['name'] = name
-        self.queue_directory = Template(section.path).safe_substitute(
-            substitutions)
+        self.queue_directory = expand(section.path, substitutions)
         numslices = int(section.instances)
         self.switchboard = Switchboard(
             self.queue_directory, slice, numslices, True)
@@ -304,7 +306,7 @@ class Runner:
         self._stop = False
 
     def __repr__(self):
-        return '<%s at %s>' % (self.__class__.__name__, id(self))
+        return '<{0} at {1:#x}>'.format(self.__class__.__name__, id(self))
 
     def stop(self):
         """See `IRunner`."""
@@ -345,13 +347,13 @@ class Runner:
                 # Ask the switchboard for the message and metadata objects
                 # associated with this queue file.
                 msg, msgdata = self.switchboard.dequeue(filebase)
-            except Exception, e:
+            except Exception as error:
                 # This used to just catch email.Errors.MessageParseError, but
                 # other problems can occur in message parsing, e.g.
                 # ValueError, and exceptions can occur in unpickling too.  We
                 # don't want the runner to die, so we just log and skip this
                 # entry, but preserve it for analysis.
-                self._log(e)
+                self._log(error)
                 elog.error('Skipping and preserving unparseable message: %s',
                            filebase)
                 self.switchboard.finish(filebase, preserve=True)
@@ -362,14 +364,14 @@ class Runner:
                 self._process_one_file(msg, msgdata)
                 dlog.debug('[%s] finishing filebase: %s', me, filebase)
                 self.switchboard.finish(filebase)
-            except Exception, e:
+            except Exception as error:
                 # All runners that implement _dispose() must guarantee that
                 # exceptions are caught and dealt with properly.  Still, there
                 # may be a bug in the infrastructure, and we do not want those
                 # to cause messages to be lost.  Any uncaught exceptions will
                 # cause the message to be stored in the shunt queue for human
                 # intervention.
-                self._log(e)
+                self._log(error)
                 # Put a marker in the metadata for unshunting.
                 msgdata['whichq'] = self.switchboard.queue_directory
                 # It is possible that shunting can throw an exception, e.g. a
@@ -380,11 +382,11 @@ class Runner:
                     new_filebase = shunt.enqueue(msg, msgdata)
                     elog.error('SHUNTING: %s', new_filebase)
                     self.switchboard.finish(filebase)
-                except Exception, e:
+                except Exception as error:
                     # The message wasn't successfully shunted.  Log the
                     # exception and try to preserve the original queue entry
                     # for possible analysis.
-                    self._log(e)
+                    self._log(error)
                     elog.error(
                         'SHUNTING FAILED, preserving original entry: %s',
                         filebase)
diff --git a/mailman/queue/docs/runner.txt b/mailman/queue/docs/runner.txt
index db493110c..d24a8334c 100644
--- a/mailman/queue/docs/runner.txt
+++ b/mailman/queue/docs/runner.txt
@@ -61,10 +61,12 @@ on instance variables.
     <BLANKLINE>
     A test message.
     <BLANKLINE>
-    >>> sorted(runner.msgdata.items())
-    [('_parsemsg', False),
-     ('bar', 'no'), ('foo', 'yes'),
-     ('lang', u'en'), ('listname', u'_xtest@example.com'),
-     ('received_time', ...), ('version', 3)]
+    >>> dump_msgdata(runner.msgdata)
+    _parsemsg: False
+    bar      : no
+    foo      : yes
+    lang     : en
+    listname : _xtest@example.com
+    version  : 3
 
 XXX More of the Runner API should be tested.
diff --git a/mailman/queue/docs/switchboard.txt b/mailman/queue/docs/switchboard.txt
index 741d435e1..88ab6ea93 100644
--- a/mailman/queue/docs/switchboard.txt
+++ b/mailman/queue/docs/switchboard.txt
@@ -29,7 +29,10 @@ Here's a helper function for ensuring things work correctly.
     ...     for qfile in os.listdir(directory):
     ...         root, ext = os.path.splitext(qfile)
     ...         files[ext] = files.get(ext, 0) + 1
-    ...     return sorted(files.items())
+    ...     if len(files) == 0:
+    ...         print 'empty'
+    ...     for ext in sorted(files):
+    ...         print '{0}: {1}'.format(ext, files[ext])
 
 
 Enqueing and dequeing
@@ -40,7 +43,7 @@ dictionary.
 
     >>> filebase = switchboard.enqueue(msg)
     >>> check_qfiles()
-    [('.pck', 1)]
+    .pck: 1
 
 To read the contents of a queue file, dequeue it.
 
@@ -51,17 +54,18 @@ To read the contents of a queue file, dequeue it.
     <BLANKLINE>
     A test message.
     <BLANKLINE>
-    >>> sorted(msgdata.items())
-    [('_parsemsg', False), ('received_time', ...), ('version', 3)]
+    >>> dump_msgdata(msgdata)
+    _parsemsg: False
+    version  : 3
     >>> check_qfiles()
-    [('.bak', 1)]
+    .bak: 1
 
 To complete the dequeing process, removing all traces of the message file,
 finish it (without preservation).
 
     >>> switchboard.finish(filebase)
     >>> check_qfiles()
-    []
+    empty
 
 When enqueing a file, you can provide additional metadata keys by using
 keyword arguments.
@@ -69,20 +73,21 @@ keyword arguments.
     >>> filebase = switchboard.enqueue(msg, {'foo': 1}, bar=2)
     >>> msg, msgdata = switchboard.dequeue(filebase)
     >>> switchboard.finish(filebase)
-    >>> sorted(msgdata.items())
-    [('_parsemsg', False),
-     ('bar', 2), ('foo', 1),
-     ('received_time', ...), ('version', 3)]
+    >>> dump_msgdata(msgdata)
+    _parsemsg: False
+    bar      : 2
+    foo      : 1
+    version  : 3
 
 Keyword arguments override keys from the metadata dictionary.
 
     >>> filebase = switchboard.enqueue(msg, {'foo': 1}, foo=2)
     >>> msg, msgdata = switchboard.dequeue(filebase)
     >>> switchboard.finish(filebase)
-    >>> sorted(msgdata.items())
-    [('_parsemsg', False),
-     ('foo', 2),
-     ('received_time', ...), ('version', 3)]
+    >>> dump_msgdata(msgdata)
+    _parsemsg: False
+    foo      : 2
+    version  : 3
 
 
 Iterating over files
@@ -99,7 +104,7 @@ iterate over just these files is to use the .files attribute.
     >>> sorted(switchboard.files) == filebases
     True
     >>> check_qfiles()
-    [('.pck', 3)]
+    .pck: 3
 
 You can also use the .get_files() method if you want to iterate over all the
 file bases for some other extension.
@@ -110,11 +115,11 @@ file bases for some other extension.
     >>> bakfiles == filebases
     True
     >>> check_qfiles()
-    [('.bak', 3)]
+    .bak: 3
     >>> for filebase in switchboard.get_files('.bak'):
     ...     switchboard.finish(filebase)
     >>> check_qfiles()
-    []
+    empty
 
 
 Recovering files
@@ -130,10 +135,10 @@ place.  These can be recovered when the switchboard is instantiated.
     ...     msg, msgdata = switchboard.dequeue(filebase)
     ...     # Don't call .finish()
     >>> check_qfiles()
-    [('.bak', 3)]
+    .bak: 3
     >>> switchboard_2 = Switchboard(queue_directory, recover=True)
     >>> check_qfiles()
-    [('.pck', 3)]
+    .pck: 3
 
 The files can be recovered explicitly.
 
@@ -141,10 +146,10 @@ The files can be recovered explicitly.
     ...     msg, msgdata = switchboard.dequeue(filebase)
     ...     # Don't call .finish()
     >>> check_qfiles()
-    [('.bak', 3)]
+    .bak: 3
     >>> switchboard.recover_backup_files()
     >>> check_qfiles()
-    [('.pck', 3)]
+    .pck: 3
 
 But the files will only be recovered at most three times before they are
 considered defective.  In order to prevent mail bombs and loops, once this
@@ -154,21 +159,21 @@ maximum is reached, the files will be preserved in the 'bad' queue.
     ...     msg, msgdata = switchboard.dequeue(filebase)
     ...     # Don't call .finish()
     >>> check_qfiles()
-    [('.bak', 3)]
+    .bak: 3
     >>> switchboard.recover_backup_files()
     >>> check_qfiles()
-    []
+    empty
 
     >>> bad = config.switchboards['bad']
     >>> check_qfiles(bad.queue_directory)
-    [('.psv', 3)]
+    .psv: 3
 
 Clean up
 
     >>> for file in os.listdir(bad.queue_directory):
     ...     os.remove(os.path.join(bad.queue_directory, file))
     >>> check_qfiles(bad.queue_directory)
-    []
+    empty
 
 
 Queue slices