Merge lp:~jtv/launchpad/db-bug-768342 into lp:launchpad/db-devel

[Jeroen T. Vermeulen]

[r=allenap][bug=768342] Log "live" output when generate-contents script runs apt-ftparchive.

= Summary =

The generate-contents script runs apt-ftparchive, which may take a long time. It's not very nice to save up all its log output and dump it just once at the end: we want live output.

== Proposed fix ==

Luckily this is all neatly isolated in a function called "execute." I replaced its subprocess.Popen implementation with one based on CommandSpawner (and its helpful friends for logging and result tracking).

== Pre-implementation notes ==

This was Julian's one gripe from testing the branch for bug 735621, currently landing, which is a requisit for this one. We couldn't think of any meaningful tests for it, since I just re-implemented an existing, tested function.

== Implementation details ==

CommandSpawner is really meant for running multiple commands in parallel, but there's nothing against using it for just one.

== Tests ==

{{{
./bin/test -vvc lp.archivepublisher.tests.test_generate_contents
}}}

== Demo and Q/A ==

Run the generate-contents script on dogfood (or some other sizable archive) and watch its output. It won't be exactly per-line granularity because of buffering, but there should be some output from apt-ftparchive before it completes.

= Launchpad lint =

This lint was previously present and is, as far as I can see, inevitable:

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/config/schema-lazr.conf
  lib/lp/archivepublisher/tests/test_generate_contents_files.py
  lib/lp/archivepublisher/scripts/generate_contents_files.py
  lib/canonical/launchpad/ftests/script.py
  cronscripts/publishing/gen-contents/apt_conf_dist.template
  cronscripts/publishing/gen-contents/Contents.top
  scripts/generate-contents-files.py
  cronscripts/publishing/gen-contents/apt_conf_header.template
  lib/lp/services/command_spawner.py

./lib/canonical/config/schema-lazr.conf
     536: Line exceeds 78 characters.
     619: Line exceeds 78 characters.
     995: Line exceeds 78 characters.
    1084: Line exceeds 78 characters.
./cronscripts/publishing/gen-contents/apt_conf_dist.template
       4: Line exceeds 78 characters.
       5: Line exceeds 78 characters.
./scripts/generate-contents-files.py
       8: '_pythonpath' imported but unused

[Gavin Panella (allenap)]

I like CommandSpawner :)

review: Approve

=== modified file 'lib/lp/archivepublisher/scripts/generate_contents_files.py'
--- lib/lp/archivepublisher/scripts/generate_contents_files.py	2011-04-21 14:28:35 +0000
+++ lib/lp/archivepublisher/scripts/generate_contents_files.py	2011-04-21 14:28:37 +0000
@@ -13,9 +13,13 @@
 from zope.component import getUtility
 
 from canonical.config import config
-from canonical.launchpad.ftests.script import run_command
 from lp.archivepublisher.config import getPubConfig
 from lp.registry.interfaces.distribution import IDistributionSet
+from lp.services.command_spawner import (
+    CommandSpawner,
+    OutputLineHandler,
+    ReturnCodeReceiver,
+    )
 from lp.services.scripts.base import (
     LaunchpadScript,
     LaunchpadScriptFailure,
@@ -63,18 +67,31 @@
     """Execute a shell command.
 
     :param logger: Output from the command will be logged here.
-    :param command_line: Command to execute, as a list of tokens.
+    :param command: Command to execute, as a string.
+    :param args: Optional list of arguments for `command`.
     :raises LaunchpadScriptFailure: If the command returns failure.
     """
-    if args is None:
-        description = command
-    else:
-        description = command + ' ' + ' '.join(args)
+    command_line = [command]
+    if args is not None:
+        command_line += args
+    description = ' '.join(command_line)
+
     logger.debug("Execute: %s", description)
-    retval, stdout, stderr = run_command(command, args)
-    logger.debug(stdout)
-    logger.warn(stderr)
-    if retval != 0:
+    # Some of these commands can take a long time.  Use CommandSpawner
+    # and friends to provide "live" log output.  Simpler ways of running
+    # commands tend to save it all up and then dump it at the end, or
+    # have trouble logging it as neat lines.
+    stderr_logger = OutputLineHandler(logger.warn)
+    stdout_logger = OutputLineHandler(logger.debug)
+    receiver = ReturnCodeReceiver()
+    spawner = CommandSpawner()
+    spawner.start(
+        command_line, completion_handler=receiver,
+        stderr_handler=stderr_logger, stdout_handler=stdout_logger)
+    spawner.complete()
+    stdout_logger.finalize()
+    stderr_logger.finalize()
+    if receiver.returncode != 0:
         raise LaunchpadScriptFailure(
             "Failure while running command: %s" % description)
 

=== modified file 'lib/lp/services/command_spawner.py'
--- lib/lp/services/command_spawner.py	2011-04-06 03:43:28 +0000
+++ lib/lp/services/command_spawner.py	2011-04-21 14:28:37 +0000
@@ -99,9 +99,9 @@
         :param stderr_handler: Callback to handle error output received from
             the sub-process.  Must take the output as its sole argument.  May
             be called any number of times as output comes in.
-        :param failure_handler: Callback to be invoked, exactly once, when the
-            sub-process exits.  Must take `command`'s numeric return code as
-            its sole argument.
+        :param completion_handler: Callback to be invoked, exactly once, when
+            the sub-process exits.  Must take `command`'s numeric return code
+            as its sole argument.
         """
         process = self._spawn(command)
         handlers = {
@@ -172,9 +172,9 @@
         try:
             output = pipe_file.read()
         except IOError, e:
+            # "Resource temporarily unavailable"--not an error really,
+            # just means there's nothing to read.
             if e.errno != errno.EAGAIN:
-                # "Resource temporarily unavailable"--not an error
-                # really, just means there's nothing to read.
                 raise
         else:
             if len(output) > 0: