launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/add-rabbit-service-bug-806160 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #806160 in Launchpad itself: "Get Rabbit running as a dev service in Launchpad."
  https://bugs.launchpad.net/launchpad/+bug/806160

For more details, see:
https://code.launchpad.net/~allenap/launchpad/add-rabbit-service-bug-806160/+merge/75195

This starts up a RabbitMQ server when using make run or make
run_all. It also moves the RabbitServer fixture off into its own
package.

A point of controversy: the hostname and port for the RabbitMQ service
is hard-coded into schema-lazr.conf. Rob has said that he wants this
to be dynamically allocated, and indeed rabbitfixture knows how to do
that already if you ask it, but it makes it much harder for other
processes - outside of the appserver started by make run/run_all - to
know where to connect.

I don't know of an existing way to do this so that it just works -
although I'm sure everyone has ideas - and I think it's important not
to increase development friction while we wait for it. Choosing a
fixed port is no worse than the existing hard-coded ports we have for
things like HTTP, and it gets us up and running and *developing*.

-- 
https://code.launchpad.net/~allenap/launchpad/add-rabbit-service-bug-806160/+merge/75195
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/add-rabbit-service-bug-806160 into lp:launchpad.

=== modified file 'Makefile'
--- Makefile	2011-09-10 09:01:08 +0000
+++ Makefile	2011-09-13 15:01:18 +0000
@@ -253,7 +253,7 @@
 	$(PY) cronscripts/merge-proposal-jobs.py -v
 
 run: build inplace stop
-	bin/run -r librarian,google-webservice,memcached -i $(LPCONFIG)
+	bin/run -r librarian,google-webservice,memcached,rabbitmq -i $(LPCONFIG)
 
 run.gdb:
 	echo 'run' > run.gdb
@@ -265,7 +265,7 @@
 
 run_all: build inplace stop
 	bin/run \
-	 -r librarian,sftp,forker,mailman,codebrowse,google-webservice,memcached \
+	 -r librarian,sftp,forker,mailman,codebrowse,google-webservice,memcached,rabbitmq \
 	 -i $(LPCONFIG)
 
 run_codebrowse: build

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2011-08-31 17:40:09 +0000
+++ lib/canonical/config/schema-lazr.conf	2011-09-13 15:01:18 +0000
@@ -1716,9 +1716,12 @@
 memory_profile_log:
 
 [rabbitmq]
+# Should RabbitMQ be launched by default?
+# datatype: boolean
+launch: True
 # The host:port at which RabbitMQ is listening.
 # datatype: string
-host: none
+host: localhost:56720
 # datatype: string
 userid: guest
 # datatype: string

=== modified file 'lib/canonical/launchpad/scripts/runlaunchpad.py'
--- lib/canonical/launchpad/scripts/runlaunchpad.py	2011-03-07 16:32:12 +0000
+++ lib/canonical/launchpad/scripts/runlaunchpad.py	2011-09-13 15:01:18 +0000
@@ -14,6 +14,7 @@
 import sys
 
 import fixtures
+from rabbitfixture.server import RabbitServerResources
 from zope.app.server.main import main
 
 from canonical.config import config
@@ -22,9 +23,10 @@
     make_pidfile,
     pidfile_path,
     )
+from lp.services.googlesearch import googletestservice
 from lp.services.mailman import runmailman
 from lp.services.osutils import ensure_directory_exists
-from lp.services.googlesearch import googletestservice
+from lp.services.rabbit.server import RabbitServer
 
 
 def make_abspath(path):
@@ -221,6 +223,20 @@
         process.stdin.close()
 
 
+class RabbitService(Service):
+    """A RabbitMQ service."""
+
+    @property
+    def should_launch(self):
+        return config.rabbitmq.launch
+
+    def launch(self):
+        hostname, port = config.rabbitmq.host.split(":")
+        self.server = RabbitServer(
+            RabbitServerResources(hostname=hostname, port=int(port)))
+        self.useFixture(self.server)
+
+
 def stop_process(process):
     """kill process and BLOCK until process dies.
 
@@ -245,6 +261,7 @@
     'codebrowse': CodebrowseService(),
     'google-webservice': GoogleWebService(),
     'memcached': MemcachedService(),
+    'rabbitmq': RabbitService(),
     }
 
 

=== modified file 'lib/canonical/launchpad/scripts/tests/test_runlaunchpad.py'
--- lib/canonical/launchpad/scripts/tests/test_runlaunchpad.py	2010-10-11 02:37:27 +0000
+++ lib/canonical/launchpad/scripts/tests/test_runlaunchpad.py	2011-09-13 15:01:18 +0000
@@ -152,6 +152,10 @@
         if config.google_test_service.launch:
             expected.append(SERVICES['google-webservice'])
 
+        # RabbitMQ may or may not be asked to run.
+        if config.rabbitmq.launch:
+            expected.append(SERVICES['rabbitmq'])
+
         expected = sorted(expected)
         self.assertEqual(expected, services)
 

=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2011-09-13 08:44:05 +0000
+++ lib/canonical/testing/layers.py	2011-09-13 15:01:18 +0000
@@ -143,13 +143,13 @@
 import lp.services.mail.stub
 from lp.services.memcache.client import memcache_client_factory
 from lp.services.osutils import kill_by_pidfile
+from lp.services.rabbit.server import RabbitServer
 from lp.testing import (
     ANONYMOUS,
     is_logged_in,
     login,
     logout,
     )
-from lp.testing.fixture import RabbitServer
 from lp.testing.pgsql import PgTestSetup
 
 

=== added directory 'lib/lp/services/rabbit'
=== added file 'lib/lp/services/rabbit/__init__.py'
=== added file 'lib/lp/services/rabbit/server.py'
--- lib/lp/services/rabbit/server.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/rabbit/server.py	2011-09-13 15:01:18 +0000
@@ -0,0 +1,31 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""RabbitMQ server fixture."""
+
+__metaclass__ = type
+__all__ = [
+    'RabbitServer',
+    ]
+
+from textwrap import dedent
+
+import rabbitfixture.server
+
+
+class RabbitServer(rabbitfixture.server.RabbitServer):
+    """A RabbitMQ server fixture with Launchpad-specific config.
+
+    :ivar service_config: A snippet of .ini that describes the `rabbitmq`
+        configuration.
+    """
+
+    def setUp(self):
+        super(RabbitServer, self).setUp()
+        self.config.service_config = dedent("""\
+            [rabbitmq]
+            host: localhost:%d
+            userid: guest
+            password: guest
+            virtual_host: /
+            """ % self.config.port)

=== added directory 'lib/lp/services/rabbit/tests'
=== added file 'lib/lp/services/rabbit/tests/__init__.py'
=== added file 'lib/lp/services/rabbit/tests/test_server.py'
--- lib/lp/services/rabbit/tests/test_server.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/rabbit/tests/test_server.py	2011-09-13 15:01:18 +0000
@@ -0,0 +1,36 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for lp.testing.fixture."""
+
+__metaclass__ = type
+
+from textwrap import dedent
+
+from fixtures import EnvironmentVariableFixture
+
+from canonical.testing.layers import BaseLayer
+from lp.services.rabbit.server import RabbitServer
+from lp.testing import TestCase
+
+
+class TestRabbitServer(TestCase):
+
+    layer = BaseLayer
+
+    def test_service_config(self):
+        # Rabbit needs to fully isolate itself: an existing per user
+        # .erlang.cookie has to be ignored, and ditto bogus HOME if other
+        # tests fail to cleanup.
+        self.useFixture(EnvironmentVariableFixture('HOME', '/nonsense/value'))
+
+        # RabbitServer pokes some .ini configuration into its config.
+        fixture = self.useFixture(RabbitServer())
+        expected = dedent("""\
+            [rabbitmq]
+            host: localhost:%d
+            userid: guest
+            password: guest
+            virtual_host: /
+            """ % fixture.config.port)
+        self.assertEqual(expected, fixture.config.service_config)

=== modified file 'lib/lp/testing/fixture.py'
--- lib/lp/testing/fixture.py	2011-09-05 16:39:46 +0000
+++ lib/lp/testing/fixture.py	2011-09-13 15:01:18 +0000
@@ -5,7 +5,6 @@
 
 __metaclass__ = type
 __all__ = [
-    'RabbitServer',
     'ZopeAdapterFixture',
     'ZopeEventHandlerFixture',
     'ZopeViewReplacementFixture',
@@ -13,14 +12,12 @@
 
 from ConfigParser import SafeConfigParser
 import os.path
-from textwrap import dedent
 
 from fixtures import (
     EnvironmentVariableFixture,
     Fixture,
     )
 import pgbouncer.fixture
-import rabbitfixture.server
 from zope.component import (
     getGlobalSiteManager,
     provideHandler,
@@ -36,24 +33,6 @@
 from canonical.config import config
 
 
-class RabbitServer(rabbitfixture.server.RabbitServer):
-    """A RabbitMQ server fixture with Launchpad-specific config.
-
-    :ivar service_config: A snippet of .ini that describes the `rabbitmq`
-        configuration.
-    """
-
-    def setUp(self):
-        super(RabbitServer, self).setUp()
-        self.config.service_config = dedent("""\
-            [rabbitmq]
-            host: localhost:%d
-            userid: guest
-            password: guest
-            virtual_host: /
-            """ % self.config.port)
-
-
 class PGBouncerFixture(pgbouncer.fixture.PGBouncerFixture):
     """Inserts a controllable pgbouncer instance in front of PostgreSQL.