Launchpad itself

Merge lp:~lifeless/launchpad/run into lp:launchpad

  1. run
  2. Merge into devel
Proposed by Robert Collins
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 11696
Proposed branch: lp:~lifeless/launchpad/run
Merge into: lp:launchpad
Diff against target: 282 lines (+50/-75)
3 files modified
Makefile (+0/-1)
lib/canonical/launchpad/scripts/runlaunchpad.py (+45/-62)
lib/canonical/launchpad/scripts/tests/test_runlaunchpad.py (+5/-12)
To merge this branch: bzr merge lp:~lifeless/launchpad/run
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+38091@code.launchpad.net

Commit message

Improvements and prep work for consolidating Service with TacTestSetup.

Description of the change

Improvements and prep work for consolidating Service with TacTestSetup

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Note that the changes to ignore should_launch are because some services deliberately ignore it - they don't run by default but when explicitly chosen they need to run; their 'should_launch' is defined as 'return False' : we already filter so that only wanted services are attempted, so checking for should_launch on setUp is a harmful (breaks consistency, causes more code) belts-and-braces.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

166 + """kill process and BLOCK until process dies.

Should have an initial capital.

I also wonder if stop_process should get more forceful than TERM, but well, that's something for another branch.

The changes look nice, btw.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Makefile'
--- Makefile 2010-09-07 18:15:01 +0000
+++ Makefile 2010-10-11 04:11:44 +0000
@@ -255,7 +255,6 @@
255 $(RM) thread*.request255 $(RM) thread*.request
256 bin/run -r librarian,sftp,codebrowse -i $(LPCONFIG)256 bin/run -r librarian,sftp,codebrowse -i $(LPCONFIG)
257257
258
259start_librarian: compile258start_librarian: compile
260 bin/start_librarian259 bin/start_librarian
261260
262261
=== modified file 'lib/canonical/launchpad/scripts/runlaunchpad.py'
--- lib/canonical/launchpad/scripts/runlaunchpad.py 2010-09-17 20:46:58 +0000
+++ lib/canonical/launchpad/scripts/runlaunchpad.py 2010-10-11 04:11:44 +0000
@@ -7,16 +7,18 @@
7__all__ = ['start_launchpad']7__all__ = ['start_launchpad']
88
99
10import atexit10from contextlib import nested
11import os11import os
12import signal12import signal
13import subprocess13import subprocess
14import sys14import sys
1515
16import fixtures
16from zope.app.server.main import main17from zope.app.server.main import main
1718
18from canonical.config import config19from canonical.config import config
19from lp.services.mailman import runmailman20from lp.services.mailman import runmailman
21from canonical.launchpad.daemons import tachandler
20from canonical.launchpad.testing import googletestservice22from canonical.launchpad.testing import googletestservice
21from canonical.lazr.pidfile import (23from canonical.lazr.pidfile import (
22 make_pidfile,24 make_pidfile,
@@ -28,20 +30,25 @@
28 return os.path.abspath(os.path.join(config.root, *path.split('/')))30 return os.path.abspath(os.path.join(config.root, *path.split('/')))
2931
3032
31TWISTD_SCRIPT = make_abspath('bin/twistd')33class Service(fixtures.Fixture):
32
33
34class Service(object):
3534
36 @property35 @property
37 def should_launch(self):36 def should_launch(self):
38 """Return true if this service should be launched."""37 """Return true if this service should be launched by default."""
39 return False38 return False
4039
41 def launch(self):40 def launch(self):
42 """Launch the service, but do not block."""41 """Run the service in a thread or external process.
42
43 May block long enough to kick it off, but must return control to
44 the caller without waiting for it to shutdown.
45 """
43 raise NotImplementedError46 raise NotImplementedError
4447
48 def setUp(self):
49 super(Service, self).setUp()
50 self.launch()
51
4552
46class TacFile(Service):53class TacFile(Service):
4754
@@ -56,8 +63,7 @@
56 :param pre_launch: A callable that is called before the launch63 :param pre_launch: A callable that is called before the launch
57 process.64 process.
58 """65 """
59 # No point calling super's __init__.66 super(TacFile, self).__init__()
60 # pylint: disable-msg=W0231
61 self.name = name67 self.name = name
62 self.tac_filename = tac_filename68 self.tac_filename = tac_filename
63 self.section_name = section_name69 self.section_name = section_name
@@ -80,10 +86,6 @@
80 return config[self.section_name].logfile86 return config[self.section_name].logfile
8187
82 def launch(self):88 def launch(self):
83 # Don't run the server if it wasn't asked for.
84 if not self.should_launch:
85 return
86
87 self.pre_launch()89 self.pre_launch()
8890
89 pidfile = pidfile_path(self.name)91 pidfile = pidfile_path(self.name)
@@ -91,7 +93,7 @@
91 tacfile = make_abspath(self.tac_filename)93 tacfile = make_abspath(self.tac_filename)
9294
93 args = [95 args = [
94 TWISTD_SCRIPT,96 tachandler.twistd_script,
95 "--no_save",97 "--no_save",
96 "--nodaemon",98 "--nodaemon",
97 "--python", tacfile,99 "--python", tacfile,
@@ -109,15 +111,8 @@
109 # ability to cycle the log files by sending a signal to the twisted111 # ability to cycle the log files by sending a signal to the twisted
110 # process.112 # process.
111 process = subprocess.Popen(args, stdin=subprocess.PIPE)113 process = subprocess.Popen(args, stdin=subprocess.PIPE)
114 self.addCleanup(stop_process, process)
112 process.stdin.close()115 process.stdin.close()
113 # I've left this off - we still check at termination and we can
114 # avoid the startup delay. -- StuartBishop 20050525
115 #time.sleep(1)
116 #if process.poll() != None:
117 # raise RuntimeError(
118 # "%s did not start: %d"
119 # % (self.name, process.returncode))
120 stop_at_exit(process)
121116
122117
123class MailmanService(Service):118class MailmanService(Service):
@@ -127,11 +122,8 @@
127 return config.mailman.launch122 return config.mailman.launch
128123
129 def launch(self):124 def launch(self):
130 # Don't run the server if it wasn't asked for. Also, don't attempt to125 runmailman.start_mailman()
131 # shut it down at exit.126 self.addCleanup(runmailman.stop_mailman)
132 if self.should_launch:
133 runmailman.start_mailman()
134 atexit.register(runmailman.stop_mailman)
135127
136128
137class CodebrowseService(Service):129class CodebrowseService(Service):
@@ -144,8 +136,8 @@
144 process = subprocess.Popen(136 process = subprocess.Popen(
145 ['make', 'run_codebrowse'],137 ['make', 'run_codebrowse'],
146 stdin=subprocess.PIPE)138 stdin=subprocess.PIPE)
139 self.addCleanup(stop_process, process)
147 process.stdin.close()140 process.stdin.close()
148 stop_at_exit(process)
149141
150142
151class GoogleWebService(Service):143class GoogleWebService(Service):
@@ -155,9 +147,7 @@
155 return config.google_test_service.launch147 return config.google_test_service.launch
156148
157 def launch(self):149 def launch(self):
158 if self.should_launch:150 self.addCleanup(stop_process, googletestservice.start_as_process())
159 process = googletestservice.start_as_process()
160 stop_at_exit(process)
161151
162152
163class MemcachedService(Service):153class MemcachedService(Service):
@@ -180,23 +170,18 @@
180 else:170 else:
181 cmd.append('-v')171 cmd.append('-v')
182 process = subprocess.Popen(cmd, stdin=subprocess.PIPE)172 process = subprocess.Popen(cmd, stdin=subprocess.PIPE)
173 self.addCleanup(stop_process, process)
183 process.stdin.close()174 process.stdin.close()
184 stop_at_exit(process)175
185176
186177def stop_process(process):
187def stop_at_exit(process):178 """kill process and BLOCK until process dies.
188 """Create and register an atexit hook for killing a process.
189
190 The hook will BLOCK until the process dies.
191179
192 :param process: An instance of subprocess.Popen.180 :param process: An instance of subprocess.Popen.
193 """181 """
194182 if process.poll() is None:
195 def stop_process():183 os.kill(process.pid, signal.SIGTERM)
196 if process.poll() is None:184 process.wait()
197 os.kill(process.pid, signal.SIGTERM)
198 process.wait()
199 atexit.register(stop_process)
200185
201186
202def prepare_for_librarian():187def prepare_for_librarian():
@@ -273,26 +258,24 @@
273 services, argv = split_out_runlaunchpad_arguments(argv[1:])258 services, argv = split_out_runlaunchpad_arguments(argv[1:])
274 argv = process_config_arguments(argv)259 argv = process_config_arguments(argv)
275 services = get_services_to_run(services)260 services = get_services_to_run(services)
276 for service in services:
277 service.launch()
278
279 # Store our process id somewhere
280 make_pidfile('launchpad')
281
282 # Create the ZCML override file based on the instance.261 # Create the ZCML override file based on the instance.
283 config.generate_overrides()262 config.generate_overrides()
284263
285 if config.launchpad.launch:264 with nested(*services):
286 main(argv)265 # Store our process id somewhere
287 else:266 make_pidfile('launchpad')
288 # We just need the foreground process to sit around forever waiting267
289 # for the signal to shut everything down. Normally, Zope itself would268 if config.launchpad.launch:
290 # be this master process, but we're not starting that up, so we need269 main(argv)
291 # to do something else.270 else:
292 try:271 # We just need the foreground process to sit around forever waiting
293 signal.pause()272 # for the signal to shut everything down. Normally, Zope itself would
294 except KeyboardInterrupt:273 # be this master process, but we're not starting that up, so we need
295 pass274 # to do something else.
275 try:
276 signal.pause()
277 except KeyboardInterrupt:
278 pass
296279
297280
298def start_librarian():281def start_librarian():
@@ -303,7 +286,7 @@
303 prepare_for_librarian()286 prepare_for_librarian()
304 pidfile = pidfile_path('librarian')287 pidfile = pidfile_path('librarian')
305 cmd = [288 cmd = [
306 TWISTD_SCRIPT,289 tachandler.twistd_script,
307 "--python", 'daemons/librarian.tac',290 "--python", 'daemons/librarian.tac',
308 "--pidfile", pidfile,291 "--pidfile", pidfile,
309 "--prefix", 'Librarian',292 "--prefix", 'Librarian',
310293
=== modified file 'lib/canonical/launchpad/scripts/tests/test_runlaunchpad.py'
--- lib/canonical/launchpad/scripts/tests/test_runlaunchpad.py 2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/scripts/tests/test_runlaunchpad.py 2010-10-11 04:11:44 +0000
@@ -13,7 +13,8 @@
13import os13import os
14import shutil14import shutil
15import tempfile15import tempfile
16import unittest16
17import testtools
1718
18import canonical.config19import canonical.config
19from canonical.config import config20from canonical.config import config
@@ -119,12 +120,12 @@
119 self.assertEquals('test', config.instance_name)120 self.assertEquals('test', config.instance_name)
120121
121122
122class ServersToStart(unittest.TestCase):123class ServersToStart(testtools.TestCase):
123 """Test server startup control."""124 """Test server startup control."""
124125
125 def setUp(self):126 def setUp(self):
126 """Make sure that only the Librarian is configured to launch."""127 """Make sure that only the Librarian is configured to launch."""
127 unittest.TestCase.setUp(self)128 testtools.TestCase.setUp(self)
128 launch_data = """129 launch_data = """
129 [librarian_server]130 [librarian_server]
130 launch: True131 launch: True
@@ -134,11 +135,7 @@
134 launch: False135 launch: False
135 """136 """
136 config.push('launch_data', launch_data)137 config.push('launch_data', launch_data)
137138 self.addCleanup(config.pop, 'launch_data')
138 def tearDown(self):
139 """Restore the default configuration."""
140 config.pop('launch_data')
141 unittest.TestCase.tearDown(self)
142139
143 def test_nothing_explictly_requested(self):140 def test_nothing_explictly_requested(self):
144 """Implicitly start services based on the config.*.launch property.141 """Implicitly start services based on the config.*.launch property.
@@ -167,7 +164,3 @@
167164
168 def test_launchpad_systems_red(self):165 def test_launchpad_systems_red(self):
169 self.failIf(config.launchpad.launch)166 self.failIf(config.launchpad.launch)
170
171
172def test_suite():
173 return unittest.TestLoader().loadTestsFromName(__name__)