Launchpad itself

Merge lp:~mwhudson/launchpad/reduce-concurrent-job-count into lp:launchpad

  1. reduce-concurrent-job-count
  2. Merge into devel
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~mwhudson/launchpad/reduce-concurrent-job-count
Merge into: lp:launchpad
Diff against target: 492 lines (+93/-79)
15 files modified
cronscripts/code-import-dispatcher.py (+8/-1)
lib/canonical/config/schema-lazr.conf (+1/-1)
lib/lp/code/doc/xmlrpc-codeimport-scheduler.txt (+3/-6)
lib/lp/code/interfaces/codeimportjob.py (+1/-1)
lib/lp/code/interfaces/codeimportmachine.py (+1/-1)
lib/lp/code/interfaces/codeimportscheduler.py (+1/-1)
lib/lp/code/model/codeimportjob.py (+2/-2)
lib/lp/code/model/codeimportmachine.py (+2/-3)
lib/lp/code/model/tests/test_codeimport.py (+1/-1)
lib/lp/code/model/tests/test_codeimportjob.py (+3/-3)
lib/lp/code/model/tests/test_codeimportmachine.py (+9/-11)
lib/lp/code/xmlrpc/codeimportscheduler.py (+3/-2)
lib/lp/codehosting/codeimport/dispatcher.py (+4/-2)
lib/lp/codehosting/codeimport/tests/test_dispatcher.py (+53/-43)
lib/lp/codehosting/codeimport/tests/test_workermonitor.py (+1/-1)
To merge this branch: bzr merge lp:~mwhudson/launchpad/reduce-concurrent-job-count
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Michael Nelson (community) code Needs Information
Review via email: mp+19574@code.launchpad.net

Commit message

Allow a code import machine to set its own limit for how many jobs to run, and lower the default to 3.

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

As discussed in the linked bug, we should probably do something more sophisticated here to give a machine a reasonable number of jobs to run, but this is much easier for sure.

Revision history for this message
Michael Nelson (michael.nelson) wrote :

Hi Michael,

doesn't this need to be a change on the lp-production-configs? Currently there I see:

production/launchpad-lazr.conf
55:max_jobs_per_machine: 2

review: Needs Information (code)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Hm, I don't think that config setting takes effect at the moment. I'll investigate further!

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

Yeah, it doesn't apply, so I'll remove it: https://code.edge.launchpad.net/~mwhudson/lp-production-configs/remove-production-max_workers_per_machine/+merge/19671

Revision history for this message
Tim Penhey (thumper) :
review: Approve
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

OK, so as discussed in IRC, I've made the worker limit settable on the code-import-dispatcher command line, and lowered the default to 3. Please re-review.

Revision history for this message
Tim Penhey (thumper) wrote :

lib/lp/codehosting/codeimport/dispatcher.py

You have:

    if worker_limit is None:
        self.worker_limit = \
            config.codeimportdispatcher.max_jobs_per_machine

Which doesn't set self.worker_limit if worker_limit is passed in.

    if worker_limit is None:
        worker_limit = config.codeimportdispatcher.max_jobs_per_machine
    self.worker_limit = worker_limit

perhaps.

How hard would it be to add an int Option? That way the OptionParser object does the integer validation and conversion for you.

  --max-jobs

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

Tim,

can you rereview now please?

Cheers,
mwh

Revision history for this message
Tim Penhey (thumper) wrote :

Looks good now

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cronscripts/code-import-dispatcher.py'
--- cronscripts/code-import-dispatcher.py 2009-10-13 14:38:07 +0000
+++ cronscripts/code-import-dispatcher.py 2010-02-22 19:42:20 +0000
@@ -18,6 +18,12 @@
1818
19class CodeImportDispatcherScript(LaunchpadScript):19class CodeImportDispatcherScript(LaunchpadScript):
2020
21 def add_my_options(self):
22 self.parser.add_option(
23 "--max-jobs", dest="max_jobs", type=int,
24 default=config.codeimportdispatcher.max_jobs_per_machine,
25 help="The maximum number of jobs to run on this machine.")
26
21 def run(self, use_web_security=False, implicit_begin=True,27 def run(self, use_web_security=False, implicit_begin=True,
22 isolation=None):28 isolation=None):
23 """See `LaunchpadScript.run`.29 """See `LaunchpadScript.run`.
@@ -30,7 +36,8 @@
30 def main(self):36 def main(self):
31 globalErrorUtility.configure('codeimportdispatcher')37 globalErrorUtility.configure('codeimportdispatcher')
3238
33 CodeImportDispatcher(self.logger).findAndDispatchJob(39 dispatcher = CodeImportDispatcher(self.logger, self.options.max_jobs)
40 dispatcher.findAndDispatchJob(
34 ServerProxy(config.codeimportdispatcher.codeimportscheduler_url))41 ServerProxy(config.codeimportdispatcher.codeimportscheduler_url))
3542
3643
3744
=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf 2010-02-20 04:13:47 +0000
+++ lib/canonical/config/schema-lazr.conf 2010-02-22 19:42:20 +0000
@@ -449,7 +449,7 @@
449codeimportscheduler_url: http://xmlrpc-private.launchpad.dev:8087/codeimportscheduler449codeimportscheduler_url: http://xmlrpc-private.launchpad.dev:8087/codeimportscheduler
450450
451# The maximum number of jobs to run on a machine at one time.451# The maximum number of jobs to run on a machine at one time.
452max_jobs_per_machine: 10452max_jobs_per_machine: 3
453453
454# See [error_reports].454# See [error_reports].
455copy_to_zlog: False455copy_to_zlog: False
456456
=== modified file 'lib/lp/code/doc/xmlrpc-codeimport-scheduler.txt'
--- lib/lp/code/doc/xmlrpc-codeimport-scheduler.txt 2009-10-22 11:55:51 +0000
+++ lib/lp/code/doc/xmlrpc-codeimport-scheduler.txt 2010-02-22 19:42:20 +0000
@@ -32,12 +32,9 @@
32getJobForMachine(), that returns the id of the job that the code32getJobForMachine(), that returns the id of the job that the code
33import slave should next run.33import slave should next run.
3434
35 >>> codeimportscheduler_api.getJobForMachine('bazaar-importer')35 >>> codeimportscheduler_api.getJobForMachine('bazaar-importer', 2)
36 136 1
3737
38 >>> from canonical.database.sqlbase import flush_database_updates
39 >>> flush_database_updates()
40
41The method just calls the 'getJobForMachine' method from the38The method just calls the 'getJobForMachine' method from the
42ICodeImportJobSet interface, and tests all the details of what it does39ICodeImportJobSet interface, and tests all the details of what it does
43can be found in the tests for ICodeImportJobSet.40can be found in the tests for ICodeImportJobSet.
@@ -49,7 +46,7 @@
49 >>> codeimportscheduler = xmlrpclib.ServerProxy(46 >>> codeimportscheduler = xmlrpclib.ServerProxy(
50 ... 'http://xmlrpc-private.launchpad.dev:8087/codeimportscheduler',47 ... 'http://xmlrpc-private.launchpad.dev:8087/codeimportscheduler',
51 ... transport=XMLRPCTestTransport())48 ... transport=XMLRPCTestTransport())
52 >>> codeimportscheduler.getJobForMachine('bazaar-importer')49 >>> codeimportscheduler.getJobForMachine('bazaar-importer', 2)
53 050 0
5451
55This includes the behaviour of auto-creating machine rows for52This includes the behaviour of auto-creating machine rows for
@@ -60,7 +57,7 @@
60 >>> print getUtility(ICodeImportMachineSet).getByHostname(57 >>> print getUtility(ICodeImportMachineSet).getByHostname(
61 ... 'doesnt-exist-yet')58 ... 'doesnt-exist-yet')
62 None59 None
63 >>> codeimportscheduler.getJobForMachine('doesnt-exist-yet')60 >>> codeimportscheduler.getJobForMachine('doesnt-exist-yet', 1)
64 061 0
65 >>> new_machine = getUtility(ICodeImportMachineSet).getByHostname(62 >>> new_machine = getUtility(ICodeImportMachineSet).getByHostname(
66 ... 'doesnt-exist-yet')63 ... 'doesnt-exist-yet')
6764
=== modified file 'lib/lp/code/interfaces/codeimportjob.py'
--- lib/lp/code/interfaces/codeimportjob.py 2009-06-25 04:06:00 +0000
+++ lib/lp/code/interfaces/codeimportjob.py 2010-02-22 19:42:20 +0000
@@ -122,7 +122,7 @@
122 # we implement endpoint specific authentication for the private xml-rpc122 # we implement endpoint specific authentication for the private xml-rpc
123 # server.123 # server.
124124
125 def getJobForMachine(hostname):125 def getJobForMachine(hostname, worker_limit):
126 """Select a job for the given machine to run and mark it as started.126 """Select a job for the given machine to run and mark it as started.
127127
128 If there is not already a CodeImportMachine with the given hostname,128 If there is not already a CodeImportMachine with the given hostname,
129129
=== modified file 'lib/lp/code/interfaces/codeimportmachine.py'
--- lib/lp/code/interfaces/codeimportmachine.py 2009-06-25 04:06:00 +0000
+++ lib/lp/code/interfaces/codeimportmachine.py 2010-02-22 19:42:20 +0000
@@ -47,7 +47,7 @@
47 description=_("When the controller deamon last recorded it was"47 description=_("When the controller deamon last recorded it was"
48 " running."))48 " running."))
4949
50 def shouldLookForJob():50 def shouldLookForJob(worker_limit):
51 """Should we look for a job to run on this machine?51 """Should we look for a job to run on this machine?
5252
53 There are three reasons we might not look for a job:53 There are three reasons we might not look for a job:
5454
=== modified file 'lib/lp/code/interfaces/codeimportscheduler.py'
--- lib/lp/code/interfaces/codeimportscheduler.py 2009-06-25 04:06:00 +0000
+++ lib/lp/code/interfaces/codeimportscheduler.py 2010-02-22 19:42:20 +0000
@@ -28,7 +28,7 @@
28 when they need more work to do.28 when they need more work to do.
29 """29 """
3030
31 def getJobForMachine(hostname):31 def getJobForMachine(hostname, worker_limit):
32 """Get a job to run on the slave 'hostname'.32 """Get a job to run on the slave 'hostname'.
3333
34 This method selects the most appropriate job for the machine,34 This method selects the most appropriate job for the machine,
3535
=== modified file 'lib/lp/code/model/codeimportjob.py'
--- lib/lp/code/model/codeimportjob.py 2010-01-27 02:48:13 +0000
+++ lib/lp/code/model/codeimportjob.py 2010-02-22 19:42:20 +0000
@@ -102,7 +102,7 @@
102 except SQLObjectNotFound:102 except SQLObjectNotFound:
103 return None103 return None
104104
105 def getJobForMachine(self, hostname):105 def getJobForMachine(self, hostname, worker_limit):
106 """See `ICodeImportJobSet`."""106 """See `ICodeImportJobSet`."""
107 job_workflow = getUtility(ICodeImportJobWorkflow)107 job_workflow = getUtility(ICodeImportJobWorkflow)
108 for job in self.getReclaimableJobs():108 for job in self.getReclaimableJobs():
@@ -111,7 +111,7 @@
111 if machine is None:111 if machine is None:
112 machine = getUtility(ICodeImportMachineSet).new(112 machine = getUtility(ICodeImportMachineSet).new(
113 hostname, CodeImportMachineState.ONLINE)113 hostname, CodeImportMachineState.ONLINE)
114 elif not machine.shouldLookForJob():114 elif not machine.shouldLookForJob(worker_limit):
115 return None115 return None
116 job = CodeImportJob.selectOne(116 job = CodeImportJob.selectOne(
117 """id IN (SELECT id FROM CodeImportJob117 """id IN (SELECT id FROM CodeImportJob
118118
=== modified file 'lib/lp/code/model/codeimportmachine.py'
--- lib/lp/code/model/codeimportmachine.py 2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/codeimportmachine.py 2010-02-22 19:42:20 +0000
@@ -51,7 +51,7 @@
51 'CodeImportEvent', joinColumn='machine',51 'CodeImportEvent', joinColumn='machine',
52 orderBy=['-date_created', '-id'])52 orderBy=['-date_created', '-id'])
5353
54 def shouldLookForJob(self):54 def shouldLookForJob(self, worker_limit):
55 """See `ICodeImportMachine`."""55 """See `ICodeImportMachine`."""
56 job_count = self.current_jobs.count()56 job_count = self.current_jobs.count()
5757
@@ -64,8 +64,7 @@
64 CodeImportMachineOfflineReason.QUIESCED)64 CodeImportMachineOfflineReason.QUIESCED)
65 return False65 return False
66 elif self.state == CodeImportMachineState.ONLINE:66 elif self.state == CodeImportMachineState.ONLINE:
67 max_jobs = config.codeimportdispatcher.max_jobs_per_machine67 return job_count < worker_limit
68 return job_count < max_jobs
69 else:68 else:
70 raise AssertionError(69 raise AssertionError(
71 "Unknown machine state %r??" % self.state)70 "Unknown machine state %r??" % self.state)
7271
=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py 2010-02-01 03:55:59 +0000
+++ lib/lp/code/model/tests/test_codeimport.py 2010-02-22 19:42:20 +0000
@@ -195,7 +195,7 @@
195195
196 def makeApprovedImportWithRunningJob(self):196 def makeApprovedImportWithRunningJob(self):
197 code_import = self.makeApprovedImportWithPendingJob()197 code_import = self.makeApprovedImportWithPendingJob()
198 job = CodeImportJobSet().getJobForMachine('machine')198 job = CodeImportJobSet().getJobForMachine('machine', 10)
199 self.assertEqual(code_import.import_job, job)199 self.assertEqual(code_import.import_job, job)
200 return code_import200 return code_import
201201
202202
=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py 2010-02-01 03:55:59 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py 2010-02-22 19:42:20 +0000
@@ -114,7 +114,7 @@
114 def assertJobIsSelected(self, desired_job):114 def assertJobIsSelected(self, desired_job):
115 """Assert that the expected job is chosen by getJobForMachine."""115 """Assert that the expected job is chosen by getJobForMachine."""
116 observed_job = getUtility(ICodeImportJobSet).getJobForMachine(116 observed_job = getUtility(ICodeImportJobSet).getJobForMachine(
117 self.machine.hostname)117 self.machine.hostname, 10)
118 self.assert_(observed_job is not None, "No job was selected.")118 self.assert_(observed_job is not None, "No job was selected.")
119 self.assertEqual(desired_job, observed_job,119 self.assertEqual(desired_job, observed_job,
120 "Expected job not selected.")120 "Expected job not selected.")
@@ -122,7 +122,7 @@
122 def assertNoJobSelected(self):122 def assertNoJobSelected(self):
123 """Assert that no job is selected."""123 """Assert that no job is selected."""
124 observed_job = getUtility(ICodeImportJobSet).getJobForMachine(124 observed_job = getUtility(ICodeImportJobSet).getJobForMachine(
125 'machine')125 'machine', 10)
126 self.assert_(observed_job is None, "Job unexpectedly selected.")126 self.assert_(observed_job is None, "Job unexpectedly selected.")
127127
128 def test_nothingSelectedIfNothingCreated(self):128 def test_nothingSelectedIfNothingCreated(self):
@@ -274,7 +274,7 @@
274 machine = self.factory.makeCodeImportMachine(set_online=True)274 machine = self.factory.makeCodeImportMachine(set_online=True)
275 login(ANONYMOUS)275 login(ANONYMOUS)
276 getUtility(ICodeImportJobSet).getJobForMachine(276 getUtility(ICodeImportJobSet).getJobForMachine(
277 machine.hostname)277 machine.hostname, 10)
278 login_for_code_imports()278 login_for_code_imports()
279 # Now there are no reclaimable jobs.279 # Now there are no reclaimable jobs.
280 self.assertReclaimableJobs([])280 self.assertReclaimableJobs([])
281281
=== modified file 'lib/lp/code/model/tests/test_codeimportmachine.py'
--- lib/lp/code/model/tests/test_codeimportmachine.py 2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/tests/test_codeimportmachine.py 2010-02-22 19:42:20 +0000
@@ -9,7 +9,6 @@
99
10from zope.component import getUtility10from zope.component import getUtility
1111
12from canonical.config import config
13from canonical.database.constants import UTC_NOW12from canonical.database.constants import UTC_NOW
14from lp.code.enums import (13from lp.code.enums import (
15 CodeImportMachineOfflineReason, CodeImportMachineState)14 CodeImportMachineOfflineReason, CodeImportMachineState)
@@ -39,13 +38,13 @@
39 def test_machineIsOffline(self):38 def test_machineIsOffline(self):
40 # When the machine is offline, we shouldn't look for any jobs.39 # When the machine is offline, we shouldn't look for any jobs.
41 self.machine.setOffline(CodeImportMachineOfflineReason.STOPPED)40 self.machine.setOffline(CodeImportMachineOfflineReason.STOPPED)
42 self.assertFalse(self.machine.shouldLookForJob())41 self.assertFalse(self.machine.shouldLookForJob(10))
4342
44 def test_machineIsQuiescingNoJobsRunning(self):43 def test_machineIsQuiescingNoJobsRunning(self):
45 # When the machine is quiescing and no jobs are running on this44 # When the machine is quiescing and no jobs are running on this
46 # machine, we should set the machine to OFFLINE and not look for jobs.45 # machine, we should set the machine to OFFLINE and not look for jobs.
47 self.machine.setQuiescing(self.factory.makePerson())46 self.machine.setQuiescing(self.factory.makePerson())
48 self.assertFalse(self.machine.shouldLookForJob())47 self.assertFalse(self.machine.shouldLookForJob(10))
49 self.assertEqual(self.machine.state, CodeImportMachineState.OFFLINE)48 self.assertEqual(self.machine.state, CodeImportMachineState.OFFLINE)
5049
51 def test_machineIsQuiescingWithJobsRunning(self):50 def test_machineIsQuiescingWithJobsRunning(self):
@@ -53,20 +52,19 @@
53 # machine, we shouldn't look for any more jobs.52 # machine, we shouldn't look for any more jobs.
54 self.createJobRunningOnMachine(self.machine)53 self.createJobRunningOnMachine(self.machine)
55 self.machine.setQuiescing(self.factory.makePerson())54 self.machine.setQuiescing(self.factory.makePerson())
56 self.assertFalse(self.machine.shouldLookForJob())55 self.assertFalse(self.machine.shouldLookForJob(10))
57 self.assertEqual(self.machine.state, CodeImportMachineState.QUIESCING)56 self.assertEqual(self.machine.state, CodeImportMachineState.QUIESCING)
5857
59 def test_enoughJobsRunningOnMachine(self):58 def test_enoughJobsRunningOnMachine(self):
60 # When there are already enough jobs running on this machine, we59 # When there are already enough jobs running on this machine, we
61 # shouldn't look for any more jobs.60 # shouldn't look for any more jobs.
62 for i in range(config.codeimportdispatcher.max_jobs_per_machine):61 self.createJobRunningOnMachine(self.machine)
63 self.createJobRunningOnMachine(self.machine)62 self.assertFalse(self.machine.shouldLookForJob(worker_limit=1))
64 self.assertFalse(self.machine.shouldLookForJob())
6563
66 def test_shouldLook(self):64 def test_shouldLook(self):
67 # If the machine is online and there are not already65 # If the machine is online and there are not already
68 # max_jobs_per_machine jobs running, then we should look for a job.66 # max_jobs_per_machine jobs running, then we should look for a job.
69 self.assertTrue(self.machine.shouldLookForJob())67 self.assertTrue(self.machine.shouldLookForJob(worker_limit=1))
7068
71 def test_noHeartbeatWhenCreated(self):69 def test_noHeartbeatWhenCreated(self):
72 # Machines are created with a NULL heartbeat.70 # Machines are created with a NULL heartbeat.
@@ -75,18 +73,18 @@
75 def test_noHeartbeatUpdateWhenOffline(self):73 def test_noHeartbeatUpdateWhenOffline(self):
76 # When the machine is offline, the heartbeat is not updated.74 # When the machine is offline, the heartbeat is not updated.
77 self.machine.setOffline(CodeImportMachineOfflineReason.STOPPED)75 self.machine.setOffline(CodeImportMachineOfflineReason.STOPPED)
78 self.machine.shouldLookForJob()76 self.machine.shouldLookForJob(10)
79 self.assertTrue(self.machine.heartbeat is None)77 self.assertTrue(self.machine.heartbeat is None)
8078
81 def test_heartbeatUpdateWhenQuiescing(self):79 def test_heartbeatUpdateWhenQuiescing(self):
82 # When the machine is quiescing, the heartbeat is updated.80 # When the machine is quiescing, the heartbeat is updated.
83 self.machine.setQuiescing(self.factory.makePerson())81 self.machine.setQuiescing(self.factory.makePerson())
84 self.machine.shouldLookForJob()82 self.machine.shouldLookForJob(10)
85 self.assertSqlAttributeEqualsDate(self.machine, 'heartbeat', UTC_NOW)83 self.assertSqlAttributeEqualsDate(self.machine, 'heartbeat', UTC_NOW)
8684
87 def test_heartbeatUpdateWhenOnline(self):85 def test_heartbeatUpdateWhenOnline(self):
88 # When the machine is online, the heartbeat is updated.86 # When the machine is online, the heartbeat is updated.
89 self.machine.shouldLookForJob()87 self.machine.shouldLookForJob(10)
90 self.assertSqlAttributeEqualsDate(self.machine, 'heartbeat', UTC_NOW)88 self.assertSqlAttributeEqualsDate(self.machine, 'heartbeat', UTC_NOW)
9189
9290
9391
=== modified file 'lib/lp/code/xmlrpc/codeimportscheduler.py'
--- lib/lp/code/xmlrpc/codeimportscheduler.py 2009-06-25 04:06:00 +0000
+++ lib/lp/code/xmlrpc/codeimportscheduler.py 2010-02-22 19:42:20 +0000
@@ -22,9 +22,10 @@
2222
23 implements(ICodeImportScheduler)23 implements(ICodeImportScheduler)
2424
25 def getJobForMachine(self, hostname):25 def getJobForMachine(self, hostname, worker_limit):
26 """See `ICodeImportScheduler`."""26 """See `ICodeImportScheduler`."""
27 job = getUtility(ICodeImportJobSet).getJobForMachine(hostname)27 job = getUtility(ICodeImportJobSet).getJobForMachine(
28 hostname, worker_limit)
28 if job is not None:29 if job is not None:
29 return job.id30 return job.id
30 else:31 else:
3132
=== modified file 'lib/lp/codehosting/codeimport/dispatcher.py'
--- lib/lp/codehosting/codeimport/dispatcher.py 2009-06-25 04:06:00 +0000
+++ lib/lp/codehosting/codeimport/dispatcher.py 2010-02-22 19:42:20 +0000
@@ -32,12 +32,13 @@
32 worker_script = os.path.join(32 worker_script = os.path.join(
33 config.root, 'scripts', 'code-import-worker-db.py')33 config.root, 'scripts', 'code-import-worker-db.py')
3434
35 def __init__(self, logger):35 def __init__(self, logger, worker_limit):
36 """Initialize an instance.36 """Initialize an instance.
3737
38 :param logger: A `Logger` object.38 :param logger: A `Logger` object.
39 """39 """
40 self.logger = logger40 self.logger = logger
41 self.worker_limit = worker_limit
4142
42 def getHostname(self):43 def getHostname(self):
43 """Return the hostname of this machine.44 """Return the hostname of this machine.
@@ -66,7 +67,8 @@
66 def findAndDispatchJob(self, scheduler_client):67 def findAndDispatchJob(self, scheduler_client):
67 """Check for and dispatch a job if necessary."""68 """Check for and dispatch a job if necessary."""
6869
69 job_id = scheduler_client.getJobForMachine(self.getHostname())70 job_id = scheduler_client.getJobForMachine(
71 self.getHostname(), self.worker_limit)
7072
71 if job_id == 0:73 if job_id == 0:
72 self.logger.info("No jobs pending.")74 self.logger.info("No jobs pending.")
7375
=== modified file 'lib/lp/codehosting/codeimport/tests/test_dispatcher.py'
--- lib/lp/codehosting/codeimport/tests/test_dispatcher.py 2009-12-14 18:11:07 +0000
+++ lib/lp/codehosting/codeimport/tests/test_dispatcher.py 2010-02-22 19:42:20 +0000
@@ -11,64 +11,60 @@
11import shutil11import shutil
12import socket12import socket
13import tempfile13import tempfile
14from textwrap import dedent
15from unittest import TestLoader14from unittest import TestLoader
1615
17from twisted.trial.unittest import TestCase
18
19from canonical.config import config
20from lp.codehosting.codeimport.dispatcher import CodeImportDispatcher
21from canonical.launchpad import scripts16from canonical.launchpad import scripts
22from canonical.launchpad.scripts.logger import QuietFakeLogger17from canonical.launchpad.scripts.logger import QuietFakeLogger
23from canonical.testing.layers import TwistedLaunchpadZopelessLayer18from canonical.testing.layers import BaseLayer
19
20from lp.codehosting.codeimport.dispatcher import CodeImportDispatcher
21from lp.testing import TestCase
2422
2523
26class StubSchedulerClient:24class StubSchedulerClient:
27 """A stub scheduler client that returns a pre-arranged answer."""25 """A scheduler client that returns a pre-arranged answer."""
2826
29 def __init__(self, id_to_return):27 def __init__(self, id_to_return):
30 self.id_to_return = id_to_return28 self.id_to_return = id_to_return
3129
32 def getJobForMachine(self, machine):30 def getJobForMachine(self, machine, limit):
33 return self.id_to_return31 return self.id_to_return
3432
3533
34class MockSchedulerClient:
35 """A scheduler client that records calls to `getJobForMachine`."""
36
37 def __init__(self):
38 self.calls = []
39
40 def getJobForMachine(self, machine, limit):
41 self.calls.append((machine, limit))
42 return 0
43
44
36class TestCodeImportDispatcherUnit(TestCase):45class TestCodeImportDispatcherUnit(TestCase):
37 """Unit tests for `CodeImportDispatcher`."""46 """Unit tests for `CodeImportDispatcher`."""
3847
39 layer = TwistedLaunchpadZopelessLayer48 layer = BaseLayer
4049
41 def setUp(self):50 def setUp(self):
42 self.config_count = 051 TestCase.setUp(self)
43 self.pushConfig(forced_hostname='none')52 self.pushConfig('codeimportdispatcher', forced_hostname='none')
44 self.dispatcher = CodeImportDispatcher(QuietFakeLogger())53
4554 def makeDispatcher(self, worker_limit=10):
46 def pushConfig(self, **args):55 """Make a `CodeImportDispatcher`."""
47 """Push some key-value pairs into the codeimportdispatcher config.56 return CodeImportDispatcher(QuietFakeLogger(), worker_limit)
48
49 The config values will be restored during test tearDown.
50 """
51 self.config_count += 1
52 name = 'test%d' % self.config_count
53 body = '\n'.join(["%s: %s"%(k, v) for k, v in args.iteritems()])
54 config.push(name, dedent("""
55 [codeimportdispatcher]
56 %s
57 """ % body))
58 self.addCleanup(config.pop, name)
5957
60 def test_getHostname(self):58 def test_getHostname(self):
61 # By default, getHostname return the same as socket.gethostname()59 # By default, getHostname return the same as socket.gethostname()
62 self.assertEqual(60 dispatcher = self.makeDispatcher()
63 self.dispatcher.getHostname(),61 self.assertEqual(socket.gethostname(), dispatcher.getHostname())
64 socket.gethostname())
6562
66 def test_getHostnameOverride(self):63 def test_getHostnameOverride(self):
67 # getHostname can be overriden by the config for testing, however.64 # getHostname can be overriden by the config for testing, however.
68 self.pushConfig(forced_hostname='test-value')65 dispatcher = self.makeDispatcher()
69 self.assertEqual(66 self.pushConfig('codeimportdispatcher', forced_hostname='test-value')
70 self.dispatcher.getHostname(),67 self.assertEqual('test-value', dispatcher.getHostname())
71 'test-value')
7268
73 def writePythonScript(self, script_path, script_body):69 def writePythonScript(self, script_path, script_body):
74 """Write out an executable Python script.70 """Write out an executable Python script.
@@ -94,6 +90,7 @@
9490
95 # We create a script that writes its command line arguments to91 # We create a script that writes its command line arguments to
96 # some a temporary file and examine that.92 # some a temporary file and examine that.
93 dispatcher = self.makeDispatcher()
97 tmpdir = tempfile.mkdtemp()94 tmpdir = tempfile.mkdtemp()
98 self.addCleanup(shutil.rmtree, tmpdir)95 self.addCleanup(shutil.rmtree, tmpdir)
99 script_path = os.path.join(tmpdir, 'script.py')96 script_path = os.path.join(tmpdir, 'script.py')
@@ -102,26 +99,39 @@
102 script_path,99 script_path,
103 ['import sys',100 ['import sys',
104 'open(%r, "w").write(str(sys.argv[1:]))' % output_path])101 'open(%r, "w").write(str(sys.argv[1:]))' % output_path])
105 self.dispatcher.worker_script = script_path102 dispatcher.worker_script = script_path
106 proc = self.dispatcher.dispatchJob(10)103 proc = dispatcher.dispatchJob(10)
107 proc.wait()104 proc.wait()
108 arglist = self.filterOutLoggingOptions(eval(open(output_path).read()))105 arglist = self.filterOutLoggingOptions(eval(open(output_path).read()))
109 self.assertEqual(arglist, ['10'])106 self.assertEqual(['10'], arglist)
110107
111 def test_findAndDispatchJob_jobWaiting(self):108 def test_findAndDispatchJob_jobWaiting(self):
112 # If there is a job to dispatch, then we call dispatchJob with its id.109 # If there is a job to dispatch, then we call dispatchJob with its id
110 # and the worker_limit supplied to the dispatcher.
113 calls = []111 calls = []
114 self.dispatcher.dispatchJob = lambda job_id: calls.append(job_id)112 dispatcher = self.makeDispatcher()
115 self.dispatcher.findAndDispatchJob(StubSchedulerClient(10))113 dispatcher.dispatchJob = lambda job_id: calls.append(job_id)
116 self.assertEqual(calls, [10])114 dispatcher.findAndDispatchJob(StubSchedulerClient(10))
115 self.assertEqual([10], calls)
117116
118 def test_findAndDispatchJob_noJobWaiting(self):117 def test_findAndDispatchJob_noJobWaiting(self):
119 # If there is no job to dispatch, then we just exit quietly.118 # If there is no job to dispatch, then we just exit quietly.
120 calls = []119 calls = []
121 self.dispatcher.dispatchJob = lambda job_id: calls.append(job_id)120 dispatcher = self.makeDispatcher()
122 self.dispatcher.findAndDispatchJob(StubSchedulerClient(0))121 dispatcher.dispatchJob = lambda job_id: calls.append(job_id)
123 self.assertEqual(calls, [])122 dispatcher.findAndDispatchJob(StubSchedulerClient(0))
123 self.assertEqual([], calls)
124124
125 def test_findAndDispatchJob_calls_getJobForMachine_with_limit(self):
126 # findAndDispatchJob calls getJobForMachine on the scheduler client
127 # with the hostname and supplied worker limit.
128 worker_limit = self.factory.getUniqueInteger()
129 dispatcher = self.makeDispatcher(worker_limit)
130 scheduler_client = MockSchedulerClient()
131 dispatcher.findAndDispatchJob(scheduler_client)
132 self.assertEqual(
133 [(dispatcher.getHostname(), worker_limit)],
134 scheduler_client.calls)
125135
126def test_suite():136def test_suite():
127 return TestLoader().loadTestsFromName(__name__)137 return TestLoader().loadTestsFromName(__name__)
128138
=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2010-02-03 19:29:27 +0000
+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2010-02-22 19:42:20 +0000
@@ -566,7 +566,7 @@
566 code_import.updateFromData(566 code_import.updateFromData(
567 {'review_status': CodeImportReviewStatus.REVIEWED},567 {'review_status': CodeImportReviewStatus.REVIEWED},
568 self.factory.makePerson())568 self.factory.makePerson())
569 job = getUtility(ICodeImportJobSet).getJobForMachine('machine')569 job = getUtility(ICodeImportJobSet).getJobForMachine('machine', 10)
570 self.assertEqual(code_import, job.code_import)570 self.assertEqual(code_import, job.code_import)
571 return job571 return job
572572