launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/bug-663828 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #663828 importing fixtures in tachandler.py breaks launchpad-buildd
  https://bugs.launchpad.net/bugs/663828


tachandler.py serves two masters: testing and production use; but TacTestSetup and ReadyService are only slightly related (they both want LOG_MAGIC); this moves ReadyService to its own file which makes it easier to maintain TacTestSetup, and fixes the (currently broken) buildd-slave packages by not needing a third party module I had depended on.
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-663828/+merge/38967
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-663828 into lp:launchpad/devel.

=== modified file 'daemons/buildd-manager.tac'
--- daemons/buildd-manager.tac	2010-07-27 10:54:18 +0000
+++ daemons/buildd-manager.tac	2010-10-20 18:46:07 +0000
@@ -11,7 +11,7 @@
 from lp.buildmaster.manager import BuilddManager
 from lp.services.twistedsupport.loggingsupport import RotatableFileLogObserver
 from canonical.config import config
-from canonical.launchpad.daemons import tachandler
+from canonical.launchpad.daemons import readyservice
 from canonical.launchpad.scripts import execute_zcml_for_scripts
 from canonical.lp import initZopeless
 
@@ -26,7 +26,7 @@
     RotatableFileLogObserver(options.get('logfile')), ignoreClass=1)
 
 # Service that announces when the daemon is ready.
-tachandler.ReadyService().setServiceParent(application)
+readyservice.ReadyService().setServiceParent(application)
 
 # Service for scanning buildd slaves.
 service = BuilddManager()

=== modified file 'daemons/buildd-slave.tac'
--- daemons/buildd-slave.tac	2010-07-26 13:54:19 +0000
+++ daemons/buildd-slave.tac	2010-10-20 18:46:07 +0000
@@ -13,7 +13,7 @@
 from canonical.buildd.sourcepackagerecipe import (
     SourcePackageRecipeBuildManager)
 from canonical.buildd.translationtemplates import TranslationTemplatesBuildManager
-from canonical.launchpad.daemons import tachandler
+from canonical.launchpad.daemons import readyservice
 
 from twisted.web import server, resource, static
 from ConfigParser import SafeConfigParser
@@ -36,7 +36,7 @@
 builddslaveService = service.IServiceCollection(application)
 
 # Service that announces when the daemon is ready
-tachandler.ReadyService().setServiceParent(builddslaveService)
+readyservice.ReadyService().setServiceParent(builddslaveService)
 
 root = resource.Resource()
 root.putChild('rpc', slave)

=== modified file 'daemons/distributionmirror_http_server.tac'
--- daemons/distributionmirror_http_server.tac	2009-06-24 20:55:31 +0000
+++ daemons/distributionmirror_http_server.tac	2010-10-20 18:46:07 +0000
@@ -9,7 +9,7 @@
 from twisted.application import service, internet, strports
 from twisted.web import server
 
-from canonical.launchpad.daemons import tachandler
+from canonical.launchpad.daemons import readyservice
 from lp.registry.tests.distributionmirror_http_server import (
     DistributionMirrorTestHTTPServer)
 
@@ -18,7 +18,7 @@
 httpserverService = service.IServiceCollection(application)
 
 # Service that announces when the daemon is ready
-tachandler.ReadyService().setServiceParent(httpserverService)
+readyservice.ReadyService().setServiceParent(httpserverService)
 
 root = DistributionMirrorTestHTTPServer()
 site = server.Site(root)

=== modified file 'daemons/librarian.tac'
--- daemons/librarian.tac	2010-07-25 05:16:58 +0000
+++ daemons/librarian.tac	2010-10-20 18:46:07 +0000
@@ -12,7 +12,7 @@
 from twisted.web import server
 
 from canonical.config import config, dbconfig
-from canonical.launchpad.daemons import tachandler
+from canonical.launchpad.daemons import readyservice
 from canonical.launchpad.scripts import execute_zcml_for_scripts
 
 from canonical.librarian.interfaces import DUMP_FILE, SIGDUMPMEM
@@ -38,7 +38,7 @@
 librarianService = service.IServiceCollection(application)
 
 # Service that announces when the daemon is ready
-tachandler.ReadyService().setServiceParent(librarianService)
+readyservice.ReadyService().setServiceParent(librarianService)
 
 def setUpListener(uploadPort, webPort, restricted):
     """Set up a librarian listener on the given ports.

=== modified file 'daemons/poppy-sftp.tac'
--- daemons/poppy-sftp.tac	2010-06-25 17:01:36 +0000
+++ daemons/poppy-sftp.tac	2010-10-20 18:46:07 +0000
@@ -17,7 +17,7 @@
 from zope.interface import implements
 
 from canonical.config import config
-from canonical.launchpad.daemons import tachandler
+from canonical.launchpad.daemons import readyservice
 
 from lp.poppy.twistedsftp import SFTPServer
 from lp.services.sshserver.auth import (
@@ -100,4 +100,4 @@
 svc.setServiceParent(application)
 
 # Service that announces when the daemon is ready
-tachandler.ReadyService().setServiceParent(application)
+readyservice.ReadyService().setServiceParent(application)

=== modified file 'daemons/sftp.tac'
--- daemons/sftp.tac	2010-04-16 18:58:24 +0000
+++ daemons/sftp.tac	2010-10-20 18:46:07 +0000
@@ -8,7 +8,7 @@
 from twisted.application import service
 
 from canonical.config import config
-from canonical.launchpad.daemons import tachandler
+from canonical.launchpad.daemons import readyservice
 
 from lp.codehosting.sshserver.daemon import (
     ACCESS_LOG_NAME, get_key_path, LOG_NAME, make_portal, OOPS_CONFIG_SECTION,
@@ -32,4 +32,4 @@
 svc.setServiceParent(application)
 
 # Service that announces when the daemon is ready
-tachandler.ReadyService().setServiceParent(application)
+readyservice.ReadyService().setServiceParent(application)

=== modified file 'daemons/zeca.tac'
--- daemons/zeca.tac	2009-06-24 20:55:31 +0000
+++ daemons/zeca.tac	2010-10-20 18:46:07 +0000
@@ -8,7 +8,7 @@
 from twisted.web import server
 
 from canonical.config import config
-from canonical.launchpad.daemons import tachandler
+from canonical.launchpad.daemons import readyservice
 from canonical.launchpad.scripts import execute_zcml_for_scripts
 from canonical.zeca import Zeca, KeyServer, LookUp, SubmitKey
 
@@ -21,7 +21,7 @@
 zecaService = service.IServiceCollection(application)
 
 # Service that announces when the daemon is ready
-tachandler.ReadyService().setServiceParent(zecaService)
+readyservice.ReadyService().setServiceParent(zecaService)
 
 zeca = Zeca()
 keyserver = KeyServer()

=== modified file 'lib/canonical/buildd/debian/rules'
--- lib/canonical/buildd/debian/rules	2010-07-26 13:54:19 +0000
+++ lib/canonical/buildd/debian/rules	2010-10-20 18:46:07 +0000
@@ -45,7 +45,7 @@
 	# Do installs here
 	touch $(pytarget)/../launchpad/__init__.py
 	touch $(pytarget)/../launchpad/daemons/__init__.py
-	cp launchpad-files/tachandler.py $(pytarget)/../launchpad/daemons/
+	cp launchpad-files/readyservice.py $(pytarget)/../launchpad/daemons/
 	install -m644 launchpad-files/buildd-slave.tac $(targetshare)/buildd-slave.tac
 	cp -r pottery $(pytarget)
 	for pyfile in $(pyfiles); do \
@@ -91,7 +91,7 @@
 
 prepare:
 	install -m644 $(daemons)/buildd-slave.tac launchpad-files/buildd-slave.tac
-	cp ../launchpad/daemons/tachandler.py launchpad-files/tachandler.py
+	cp ../launchpad/daemons/readyservice.py launchpad-files/readyservice.py
 
 package: prepare
 	debuild -uc -us -S

=== modified file 'lib/canonical/database/ftests/portforward-to-postgres.tac'
--- lib/canonical/database/ftests/portforward-to-postgres.tac	2009-06-30 21:06:27 +0000
+++ lib/canonical/database/ftests/portforward-to-postgres.tac	2010-10-20 18:46:07 +0000
@@ -10,7 +10,7 @@
 from twisted.application.internet import TCPServer
 
 import canonical.lp
-from canonical.launchpad.daemons.tachandler import ReadyService
+from canonical.launchpad.daemons.readyservice import ReadyService
 
 application = service.Application('portforward_to_postgres')
 

=== added file 'lib/canonical/launchpad/daemons/readyservice.py'
--- lib/canonical/launchpad/daemons/readyservice.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/daemons/readyservice.py	2010-10-20 18:46:07 +0000
@@ -0,0 +1,29 @@
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Add logging for when twistd services start up.
+
+Used externally to launchpad (by launchpad-buildd) - must not import
+any Launchpad code, and any new external dependencies need coordination
+with the launchpad-buildd deployment.
+"""
+
+__metaclass__ = type
+
+__all__ = [
+    'ReadyService',
+    ]
+
+from twisted.application import service
+from twisted.python import log
+
+LOG_MAGIC = 'daemon ready!'
+
+
+class ReadyService(service.Service):
+    """Service that logs a 'ready!' message once the reactor has started."""
+
+    def startService(self):
+        from twisted.internet import reactor
+        reactor.addSystemEventTrigger('after', 'startup', log.msg, LOG_MAGIC)
+        service.Service.startService(self)

=== modified file 'lib/canonical/launchpad/daemons/tachandler.py'
--- lib/canonical/launchpad/daemons/tachandler.py	2010-09-28 22:33:42 +0000
+++ lib/canonical/launchpad/daemons/tachandler.py	2010-10-20 18:46:07 +0000
@@ -9,7 +9,6 @@
 
 __all__ = [
     'TacTestSetup',
-    'ReadyService',
     'TacException',
     'kill_by_pidfile',
     'remove_if_exists',
@@ -32,8 +31,8 @@
 from twisted.application import service
 from twisted.python import log
 
-# This file is used by launchpad-buildd, so it cannot import any
-# Launchpad code!
+from canonical.launchpad.daemons import readyservice
+
 
 def _kill_may_race(pid, signal_number):
     """Kill a pid accepting that it may not exist."""
@@ -46,6 +45,7 @@
         # Some other issue (e.g. different user owns it)
         raise
 
+
 def two_stage_kill(pid, poll_interval=0.1, num_polls=50):
     """Kill process 'pid' with SIGTERM. If it doesn't die, SIGKILL it.
 
@@ -117,8 +117,6 @@
     os.path.dirname(__file__),
     os.pardir, os.pardir, os.pardir, os.pardir, 'bin', 'twistd'))
 
-LOG_MAGIC = 'daemon ready!'
-
 
 class TacException(Exception):
     """Error raised by TacTestSetup."""
@@ -147,9 +145,10 @@
                     "Could not kill stale process %s." % (self.pidfile,))
 
         # setUp() watches the logfile to determine when the daemon has fully
-        # started. If it sees an old logfile, then it will find the LOG_MAGIC
-        # string and return immediately, provoking hard-to-diagnose race
-        # conditions. Delete the logfile to make sure this does not happen.
+        # started. If it sees an old logfile, then it will find the
+        # readyservice.LOG_MAGIC string and return immediately, provoking
+        # hard-to-diagnose race conditions. Delete the logfile to make sure
+        # this does not happen.
         remove_if_exists(self.logfile)
 
         self.setUpRoot()
@@ -186,12 +185,12 @@
     def _hasDaemonStarted(self):
         """Has the daemon started?
 
-        Startup is recognized by the appearance of LOG_MAGIC in the log
-        file.
+        Startup is recognized by the appearance of readyservice.LOG_MAGIC in
+        the log file.
         """
         if os.path.exists(self.logfile):
             with open(self.logfile, 'r') as logfile:
-                return LOG_MAGIC in logfile.read()
+                return readyservice.LOG_MAGIC in logfile.read()
         else:
             return False
 
@@ -203,8 +202,8 @@
 
         :raises TacException: Timeout.
         """
-        # Watch the log file for LOG_MAGIC to signal that startup has
-        # completed.
+        # Watch the log file for readyservice.LOG_MAGIC to signal that startup
+        # has completed.
         now = time.time()
         deadline = now + 20
         while now < deadline and not self._hasDaemonStarted():
@@ -255,12 +254,3 @@
     @property
     def logfile(self):
         raise NotImplementedError
-
-
-class ReadyService(service.Service):
-    """Service that logs a 'ready!' message once the reactor has started."""
-
-    def startService(self):
-        from twisted.internet import reactor
-        reactor.addSystemEventTrigger('after', 'startup', log.msg, LOG_MAGIC)
-        service.Service.startService(self)

=== modified file 'lib/canonical/launchpad/daemons/tests/cannotlisten.tac'
--- lib/canonical/launchpad/daemons/tests/cannotlisten.tac	2009-06-30 21:06:27 +0000
+++ lib/canonical/launchpad/daemons/tests/cannotlisten.tac	2010-10-20 18:46:07 +0000
@@ -11,14 +11,14 @@
 from twisted.application import service, internet, strports
 from twisted.internet import protocol
 
-from canonical.launchpad.daemons import tachandler
+from canonical.launchpad.daemons import readyservice
 
 
 application = service.Application('CannotListen')
 serviceCollection = service.IServiceCollection(application)
 
 # Service that announces when the daemon is ready
-tachandler.ReadyService().setServiceParent(serviceCollection)
+readyservice.ReadyService().setServiceParent(serviceCollection)
 
 # We almost certainly can't listen on port 1 (usually it requires root
 # permissions), so this should fail.