launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06379
[Merge] lp:~allenap/maas/disable-rabbit-in-pserv into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/disable-rabbit-in-pserv into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/disable-rabbit-in-pserv/+merge/93210
--
https://code.launchpad.net/~allenap/maas/disable-rabbit-in-pserv/+merge/93210
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/disable-rabbit-in-pserv into lp:maas.
=== modified file 'src/provisioningserver/plugin.py'
--- src/provisioningserver/plugin.py 2012-02-10 11:48:52 +0000
+++ src/provisioningserver/plugin.py 2012-02-15 14:31:27 +0000
@@ -47,17 +47,22 @@
optParameters = [
["port", None, 8001, "Port to serve on."],
["logfile", "l", "pserv.log", "Logfile name."],
+ ["oops-dir", "r", None, "Where to write OOPS reports"],
+ ["oops-reporter", "o", "MAAS-PS", "String identifying this service."],
+ ]
+
+ # Move these back into optParameters when RabbitMQ is a required component
+ # of a running MaaS installation.
+ optParameters_FOR_RABBIT = [
["brokerport", "p", 5672, "Broker port"],
["brokerhost", "h", '127.0.0.1', "Broker host"],
["brokeruser", "u", None, "Broker user"],
["brokerpassword", "a", None, "Broker password"],
["brokervhost", "v", '/', "Broker vhost"],
- ["oops-dir", "r", None, "Where to write OOPS reports"],
- ["oops-reporter", "o", "MAAS-PS", "String identifying this service."],
]
def postOptions(self):
- for int_arg in ('port', 'brokerport'):
+ for int_arg in ('port',):
try:
self[int_arg] = int(self[int_arg])
except (TypeError, ValueError):
@@ -102,11 +107,11 @@
oops_service = OOPSService(log_service, oops_dir, oops_reporter)
oops_service.setServiceParent(services)
- broker_port = options["brokerport"]
- broker_host = options["brokerhost"]
- broker_user = options["brokeruser"]
- broker_password = options["brokerpassword"]
- broker_vhost = options["brokervhost"]
+ broker_port = options.get("brokerport")
+ broker_host = options.get("brokerhost")
+ broker_user = options.get("brokeruser")
+ broker_password = options.get("brokerpassword")
+ broker_vhost = options.get("brokervhost")
# Connecting to RabbitMQ is optional; it is not yet a required
# component of a running MaaS installation.
=== modified file 'src/provisioningserver/testing/amqpclient.py'
--- src/provisioningserver/testing/amqpclient.py 2012-02-02 23:47:54 +0000
+++ src/provisioningserver/testing/amqpclient.py 2012-02-15 14:31:27 +0000
@@ -11,6 +11,8 @@
__metaclass__ = type
__all__ = []
+from unittest import skip
+
from provisioningserver.amqpclient import AMQFactory
from testtools import TestCase
from testtools.deferredruntest import (
@@ -66,8 +68,11 @@
"""
from provisioningserver import tests
- return tests.rabbit
+ return tests.get_rabbit()
+ @skip(
+ "RabbitMQ is not yet a required component "
+ "of a running MaaS installation.")
def setUp(self):
"""
At each run, we delete the test vhost and recreate it, to be sure to be
=== modified file 'src/provisioningserver/tests/__init__.py'
--- src/provisioningserver/tests/__init__.py 2012-02-05 14:56:21 +0000
+++ src/provisioningserver/tests/__init__.py 2012-02-15 14:31:27 +0000
@@ -9,23 +9,28 @@
)
__metaclass__ = type
-__all__ = []
+__all__ = [
+ "get_rabbit",
+ ]
from rabbitfixture.server import RabbitServer
-# Set setUp() and tearDown().
+# Set get_rabbit() and tearDown().
rabbit = None
-def setUp():
- """Package-level fixture hook, recognized by nose."""
+def get_rabbit():
+ """Return a running `RabbitServer` fixture."""
global rabbit
- rabbit = RabbitServer()
- rabbit.setUp()
+ if rabbit is None:
+ rabbit = RabbitServer()
+ rabbit.setUp()
+ return rabbit
def tearDown():
"""Package-level fixture hook, recognized by nose."""
global rabbit
- rabbit.cleanUp()
- rabbit = None
+ if rabbit is not None:
+ rabbit.cleanUp()
+ rabbit = None
=== modified file 'src/provisioningserver/tests/test_plugin.py'
--- src/provisioningserver/tests/test_plugin.py 2012-02-10 11:48:52 +0000
+++ src/provisioningserver/tests/test_plugin.py 2012-02-15 14:31:27 +0000
@@ -32,7 +32,12 @@
class TestOptions(TestCase):
"""Tests for `provisioningserver.plugin.Options`."""
- def test_defaults(self):
+ @skip(
+ "RabbitMQ is not yet a required component "
+ "of a running MaaS installation. Re-enable "
+ "when RabbitMQ is once again needed; remove "
+ "the other similarly named test.")
+ def test_defaults_SKIPPED(self):
options = Options()
expected = {
"brokerhost": "127.0.0.1",
@@ -47,6 +52,16 @@
}
self.assertEqual(expected, options.defaults)
+ def test_defaults(self):
+ options = Options()
+ expected = {
+ "logfile": "pserv.log",
+ "oops-dir": None,
+ "oops-reporter": "MAAS-PS",
+ "port": 8001,
+ }
+ self.assertEqual(expected, options.defaults)
+
def check_exception(self, options, message, *arguments):
# Check that a UsageError is raised when parsing options.
self.assertThat(
@@ -78,7 +93,12 @@
arguments = []
options.parseOptions(arguments) # No error.
- def test_parse_int_options(self):
+ @skip(
+ "RabbitMQ is not yet a required component "
+ "of a running MaaS installation. Re-enable "
+ "when RabbitMQ is once again needed; remove "
+ "the other similarly named test.")
+ def test_parse_int_options_SKIPPED(self):
# Some options are converted to ints.
options = Options()
arguments = [
@@ -91,23 +111,64 @@
self.assertEqual(4321, options["brokerport"])
self.assertEqual(3456, options["port"])
+ def test_parse_int_options(self):
+ # Some options are converted to ints.
+ options = Options()
+ arguments = [
+ "--port", "3456",
+ ]
+ options.parseOptions(arguments)
+ self.assertEqual(3456, options["port"])
+
+ @skip(
+ "RabbitMQ is not yet a required component "
+ "of a running MaaS installation. Re-enable "
+ "when RabbitMQ is once again needed; remove "
+ "the other similarly named test.")
+ def test_parse_broken_int_options_SKIPPED(self):
+ # An error is raised if the integer options do not contain integers.
+ options = Options()
+ arguments = [
+ "--brokerpassword", "Hoskins",
+ "--brokerport", "Jr.",
+ "--brokeruser", "Bob",
+ ]
+ self.assertRaises(
+ UsageError, options.parseOptions, arguments)
+
def test_parse_broken_int_options(self):
# An error is raised if the integer options do not contain integers.
options = Options()
arguments = [
+ "--port", "Metallica",
+ ]
+ self.assertRaises(
+ UsageError, options.parseOptions, arguments)
+
+ @skip(
+ "RabbitMQ is not yet a required component "
+ "of a running MaaS installation. Re-enable "
+ "when RabbitMQ is once again needed; remove "
+ "the other similarly named test.")
+ def test_oops_dir_without_reporter_SKIPPED(self):
+ # It is an error to omit the OOPS reporter if directory is specified.
+ options = Options()
+ arguments = [
"--brokerpassword", "Hoskins",
- "--brokerport", "Jr.",
"--brokeruser", "Bob",
+ "--oops-dir", "/some/where",
+ "--oops-reporter", "",
]
- self.assertRaises(
- UsageError, options.parseOptions, arguments)
+ expected = MatchesException(
+ UsageError, "A reporter must be supplied")
+ self.assertThat(
+ partial(options.parseOptions, arguments),
+ Raises(expected))
def test_oops_dir_without_reporter(self):
# It is an error to omit the OOPS reporter if directory is specified.
options = Options()
arguments = [
- "--brokerpassword", "Hoskins",
- "--brokeruser", "Bob",
"--oops-dir", "/some/where",
"--oops-reporter", "",
]
@@ -148,6 +209,9 @@
site_service = service.getServiceNamed("site")
self.assertEqual(options["port"], site_service.args[0])
+ @skip(
+ "RabbitMQ is not yet a required component "
+ "of a running MaaS installation.")
def test_makeService_with_broker(self):
"""
The log, oops, site, and amqp services are created when the broker
