Launchpad itself

Merge lp:~al-maisan/launchpad/ibuilder-api-cleanup-505647 into lp:launchpad

  1. ibuilder-api-cleanup-505647
  2. Merge into devel
Proposed by Muharem Hrnjadovic on 2010-01-11
Status: Merged
Merged at revision: not available
Proposed branch: lp:~al-maisan/launchpad/ibuilder-api-cleanup-505647
Merge into: lp:launchpad
Diff against target: 622 lines (+117/-93)
7 files modified
lib/lp/buildmaster/manager.py (+2/-10)
lib/lp/soyuz/doc/buildd-dispatching.txt (+17/-11)
lib/lp/soyuz/doc/buildd-slavescanner.txt (+31/-30)
lib/lp/soyuz/interfaces/builder.py (+8/-18)
lib/lp/soyuz/model/builder.py (+41/-9)
lib/lp/soyuz/scripts/buildd.py (+2/-4)
lib/lp/soyuz/tests/test_builder.py (+16/-11)
To merge this branch: bzr merge lp:~al-maisan/launchpad/ibuilder-api-cleanup-505647
Reviewer Review Type Date Requested Status
Björn Tillenius (community) 2010-01-11 Approve on 2010-01-11
Review via email: mp+17122@code.launchpad.net
To post a comment you must log in.
Muharem Hrnjadovic (al-maisan) wrote :

Hello there!

This branch cleans up the IBuilder interface and model code related to
candidate job selection.
The findBuildCandidate()/dispatchBuildCandidate() methods are made internal
and a new findAndStartJob() method that hides them is introduced.

Tests to run:

   bin/test -vv -t build

No "make lint" errors.

Björn Tillenius (bjornt) wrote :
Download full text (3.7 KiB)

On Mon, Jan 11, 2010 at 03:08:17AM -0000, Muharem Hrnjadovic wrote:
> === modified file 'lib/lp/buildmaster/manager.py'
> --- lib/lp/buildmaster/manager.py 2009-07-26 14:19:49 +0000
> +++ lib/lp/buildmaster/manager.py 2010-01-11 03:08:16 +0000
> @@ -340,17 +340,9 @@
> transaction.commit()
> continue
>
> - candidate = builder.findBuildCandidate()
> - if candidate is None:
> - self.logger.debug(
> - "No build candidates available for builder.")
> - continue
> -
> slave = RecordingSlave(builder.name, builder.url, builder.vm_host)
> - builder.setSlaveForTesting(slave)
> -
> - builder.dispatchBuildCandidate(candidate)
> - if builder.currentjob is not None:
> + candidate = builder.findAndStartJob(buildd_slave=slave)
> + if candidate is not None:
> recording_slaves.append(slave)
> transaction.commit()

Here you changed it to commit when no candidates are found...

> === modified file 'lib/lp/soyuz/scripts/buildd.py'
> --- lib/lp/soyuz/scripts/buildd.py 2009-10-26 18:40:04 +0000
> +++ lib/lp/soyuz/scripts/buildd.py 2010-01-11 03:08:16 +0000
> @@ -201,12 +201,10 @@
> if not builder.is_available:
> self.logger.warn('builder is not available. Ignored.')
> continue
> - candidate = builder.findBuildCandidate()
> +
> + candidate = builder.findAndStartJob()
> if candidate is None:
> - self.logger.debug(
> - "No candidates available for builder.")
> continue
> - builder.dispatchBuildCandidate(candidate)
> self.txn.commit()

... but here you retain the behaviour of not commiting when no candidate
is found. Why was one place changed, but not the other?

> === modified file 'lib/lp/soyuz/tests/test_builder.py'
> --- lib/lp/soyuz/tests/test_builder.py 2009-12-02 15:18:46 +0000
> +++ lib/lp/soyuz/tests/test_builder.py 2010-01-11 03:08:16 +0000

> @@ -34,13 +35,15 @@
> # Create some i386 builders ready to build PPA builds. Two
> # already exist in sampledata so we'll use those first.
> self.builder1 = getUtility(IBuilderSet)['bob']
> - self.builder2 = getUtility(IBuilderSet)['frog']
> + self.frog_builder = removeSecurityProxy(
> + getUtility(IBuilderSet)['frog'])

Why doesn't frog_builder have a security proxy? If it's just to call
private methods, it's better to remove the proxy when you call the
method. If you start passing frog_builder to methods in your tests, you
might get things passing in the tests, while failing in real code.
Better to be safe and only unwrap the objects when you know it's safe.

> self.builder3 = self.factory.makeBuilder(name='builder3')
> - self.builder4 = self.factory.makeBuilder(name='builder4')
> + self.builder4 = removeSecurityProxy(
> + self.factory.makeBuilder(name='builder4'))

Same comment here.

> self.builder5 = self.factory.makeBuilder(name='builder5')
> self.b...

Read more...

review: Needs Information
Muharem Hrnjadovic (al-maisan) wrote :
Download full text (4.2 KiB)

Björn Tillenius wrote:
> Review: Needs Information
> On Mon, Jan 11, 2010 at 03:08:17AM -0000, Muharem Hrnjadovic wrote:
>> === modified file 'lib/lp/buildmaster/manager.py'
>> --- lib/lp/buildmaster/manager.py 2009-07-26 14:19:49 +0000
>> +++ lib/lp/buildmaster/manager.py 2010-01-11 03:08:16 +0000
>> @@ -340,17 +340,9 @@
>> transaction.commit()
>> continue
>>
>> - candidate = builder.findBuildCandidate()
>> - if candidate is None:
>> - self.logger.debug(
>> - "No build candidates available for builder.")
>> - continue
>> -
>> slave = RecordingSlave(builder.name, builder.url, builder.vm_host)
>> - builder.setSlaveForTesting(slave)
>> -
>> - builder.dispatchBuildCandidate(candidate)
>> - if builder.currentjob is not None:
>> + candidate = builder.findAndStartJob(buildd_slave=slave)
>> + if candidate is not None:
>> recording_slaves.append(slave)
>> transaction.commit()
>
> Here you changed it to commit when no candidates are found...
 Good catch! I have indented the commit() line above to preserve the old
behaviour.

>> === modified file 'lib/lp/soyuz/scripts/buildd.py'
>> --- lib/lp/soyuz/scripts/buildd.py 2009-10-26 18:40:04 +0000
>> +++ lib/lp/soyuz/scripts/buildd.py 2010-01-11 03:08:16 +0000
>> @@ -201,12 +201,10 @@
>> if not builder.is_available:
>> self.logger.warn('builder is not available. Ignored.')
>> continue
>> - candidate = builder.findBuildCandidate()
>> +
>> + candidate = builder.findAndStartJob()
>> if candidate is None:
>> - self.logger.debug(
>> - "No candidates available for builder.")
>> continue
>> - builder.dispatchBuildCandidate(candidate)
>> self.txn.commit()
>
> ... but here you retain the behaviour of not commiting when no candidate
> is found. Why was one place changed, but not the other?
The issue above is fixed now.

>> === modified file 'lib/lp/soyuz/tests/test_builder.py'
>> --- lib/lp/soyuz/tests/test_builder.py 2009-12-02 15:18:46 +0000
>> +++ lib/lp/soyuz/tests/test_builder.py 2010-01-11 03:08:16 +0000
>
>> @@ -34,13 +35,15 @@
>> # Create some i386 builders ready to build PPA builds. Two
>> # already exist in sampledata so we'll use those first.
>> self.builder1 = getUtility(IBuilderSet)['bob']
>> - self.builder2 = getUtility(IBuilderSet)['frog']
>> + self.frog_builder = removeSecurityProxy(
>> + getUtility(IBuilderSet)['frog'])
>
> Why doesn't frog_builder have a security proxy? If it's just to call
> private methods, it's better to remove the proxy when you call the
> method. If you start passing frog_builder to methods in your tests, you
> might get things passing in the tests, while failing in real code.
> Better to be safe and only unwrap the objects when you know it's safe.
Good point! Revised the code accordingly.

>> self.builder3 = self.factory.makeBuilder(name='builder3')
>> - ...

Read more...

ibuilder-api-cleanup-505647-inc.diff
1=== modified file 'lib/lp/buildmaster/manager.py'
2--- lib/lp/buildmaster/manager.py 2010-01-10 23:59:31 +0000
3+++ lib/lp/buildmaster/manager.py 2010-01-11 03:39:35 +0000
4@@ -344,7 +344,7 @@
5 candidate = builder.findAndStartJob(buildd_slave=slave)
6 if candidate is not None:
7 recording_slaves.append(slave)
8- transaction.commit()
9+ transaction.commit()
10
11 return recording_slaves
12
13
14=== modified file 'lib/lp/soyuz/tests/test_builder.py'
15--- lib/lp/soyuz/tests/test_builder.py 2010-01-11 03:03:19 +0000
16+++ lib/lp/soyuz/tests/test_builder.py 2010-01-11 03:43:47 +0000
17@@ -35,11 +35,9 @@
18 # Create some i386 builders ready to build PPA builds. Two
19 # already exist in sampledata so we'll use those first.
20 self.builder1 = getUtility(IBuilderSet)['bob']
21- self.frog_builder = removeSecurityProxy(
22- getUtility(IBuilderSet)['frog'])
23+ self.frog_builder = getUtility(IBuilderSet)['frog']
24 self.builder3 = self.factory.makeBuilder(name='builder3')
25- self.builder4 = removeSecurityProxy(
26- self.factory.makeBuilder(name='builder4'))
27+ self.builder4 = self.factory.makeBuilder(name='builder4')
28 self.builder5 = self.factory.makeBuilder(name='builder5')
29 self.builders = [
30 self.builder1,
31@@ -65,8 +63,7 @@
32 self.publisher.prepareBreezyAutotest()
33
34 self.bob_builder = getUtility(IBuilderSet)['bob']
35- self.frog_builder = removeSecurityProxy(
36- getUtility(IBuilderSet)['frog'])
37+ self.frog_builder = getUtility(IBuilderSet)['frog']
38
39 # Disable bob so only frog is available.
40 self.bob_builder.manual = True
41@@ -85,7 +82,8 @@
42 # there's only one builder available.
43
44 # Asking frog to find a candidate should give us the joesppa build.
45- next_job = self.frog_builder._findBuildCandidate()
46+ next_job = removeSecurityProxy(
47+ self.frog_builder)._findBuildCandidate()
48 build = getUtility(IBuildSet).getByQueueEntry(next_job)
49 self.assertEqual('joesppa', build.archive.name)
50
51@@ -93,7 +91,8 @@
52 # returned.
53 self.bob_builder.builderok = False
54 self.bob_builder.manual = False
55- next_job = self.frog_builder._findBuildCandidate()
56+ next_job = removeSecurityProxy(
57+ self.frog_builder)._findBuildCandidate()
58 build = getUtility(IBuildSet).getByQueueEntry(next_job)
59 self.assertEqual('joesppa', build.archive.name)
60
61@@ -166,7 +165,7 @@
62 def test_findBuildCandidate_first_build_started(self):
63 # A PPA cannot start a build if it would use 80% or more of the
64 # builders.
65- next_job = self.builder4._findBuildCandidate()
66+ next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
67 build = getUtility(IBuildSet).getByQueueEntry(next_job)
68 self.failIfEqual('joesppa', build.archive.name)
69
70@@ -174,7 +173,7 @@
71 # When joe's first ppa build finishes, his fourth i386 build
72 # will be the next build candidate.
73 self.joe_builds[0].buildstate = BuildStatus.FAILEDTOBUILD
74- next_job = self.builder4._findBuildCandidate()
75+ next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
76 build = getUtility(IBuildSet).getByQueueEntry(next_job)
77 self.failUnlessEqual('joesppa', build.archive.name)
78
79@@ -183,7 +182,7 @@
80 # for the one architecture.
81 self.ppa_joe.private = True
82 self.ppa_joe.buildd_secret = 'sekrit'
83- next_job = self.builder4._findBuildCandidate()
84+ next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
85 build = getUtility(IBuildSet).getByQueueEntry(next_job)
86 self.failUnlessEqual('joesppa', build.archive.name)
87
88@@ -209,7 +208,8 @@
89 # Normal archives are not restricted to serial builds per
90 # arch.
91
92- next_job = self.frog_builder._findBuildCandidate()
93+ next_job = removeSecurityProxy(
94+ self.frog_builder)._findBuildCandidate()
95 build = getUtility(IBuildSet).getByQueueEntry(next_job)
96 self.failUnlessEqual('primary', build.archive.name)
97 self.failUnlessEqual('gedit', build.sourcepackagerelease.name)
98@@ -218,7 +218,8 @@
99 # second non-ppa build for the same archive as the next candidate.
100 build.buildstate = BuildStatus.BUILDING
101 build.builder = self.frog_builder
102- next_job = self.frog_builder._findBuildCandidate()
103+ next_job = removeSecurityProxy(
104+ self.frog_builder)._findBuildCandidate()
105 build = getUtility(IBuildSet).getByQueueEntry(next_job)
106 self.failUnlessEqual('primary', build.archive.name)
107 self.failUnlessEqual('firefox', build.sourcepackagerelease.name)
Björn Tillenius (bjornt) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/manager.py'
2--- lib/lp/buildmaster/manager.py 2009-07-26 14:19:49 +0000
3+++ lib/lp/buildmaster/manager.py 2010-01-11 21:56:15 +0000
4@@ -340,19 +340,11 @@
5 transaction.commit()
6 continue
7
8- candidate = builder.findBuildCandidate()
9- if candidate is None:
10- self.logger.debug(
11- "No build candidates available for builder.")
12- continue
13-
14 slave = RecordingSlave(builder.name, builder.url, builder.vm_host)
15- builder.setSlaveForTesting(slave)
16-
17- builder.dispatchBuildCandidate(candidate)
18+ candidate = builder.findAndStartJob(buildd_slave=slave)
19 if builder.currentjob is not None:
20 recording_slaves.append(slave)
21- transaction.commit()
22+ transaction.commit()
23
24 return recording_slaves
25
26
27=== modified file 'lib/lp/soyuz/doc/buildd-dispatching.txt'
28--- lib/lp/soyuz/doc/buildd-dispatching.txt 2009-11-13 19:34:17 +0000
29+++ lib/lp/soyuz/doc/buildd-dispatching.txt 2010-01-11 21:56:15 +0000
30@@ -128,7 +128,8 @@
31 Now let's check the build candidates which will be considered for the
32 builder 'bob':
33
34- >>> job = bob_builder.findBuildCandidate()
35+ >>> from zope.security.proxy import removeSecurityProxy
36+ >>> job = removeSecurityProxy(bob_builder)._findBuildCandidate()
37
38 The single BuildQueue found is a non-virtual pending build:
39
40@@ -157,14 +158,16 @@
41 ... 'foo.dsc', len(content), StringIO(content), 'application/dsc')
42
43 >>> sprf = build.sourcepackagerelease.files[0]
44- >>> from zope.security.proxy import removeSecurityProxy
45 >>> naked_sprf = removeSecurityProxy(sprf)
46 >>> naked_sprf.libraryfile = getUtility(ILibraryFileAliasSet)[alias_id]
47 >>> flush_database_updates()
48
49 Check the dispatching method itself:
50
51- >>> bob_builder.dispatchBuildCandidate(job)
52+ >>> dispatched_job = bob_builder.findAndStartJob()
53+ >>> job == dispatched_job
54+ True
55+
56 >>> flush_database_updates()
57
58 Verify if the job (BuildQueue) was updated appropriately:
59@@ -224,7 +227,7 @@
60 >>> bob_builder.vm_host = 'localhost.ppa'
61 >>> syncUpdate(bob_builder)
62
63- >>> job = bob_builder.findBuildCandidate()
64+ >>> job = removeSecurityProxy(bob_builder)._findBuildCandidate()
65 >>> print job
66 None
67
68@@ -245,11 +248,11 @@
69 >>> bob_builder.virtualized = True
70 >>> syncUpdate(bob_builder)
71
72- >>> job = bob_builder.findBuildCandidate()
73+ >>> job = removeSecurityProxy(bob_builder)._findBuildCandidate()
74 >>> ppa_job.id == job.id
75 True
76
77-For further details regarding IBuilder.findBuildCandidate() please see
78+For further details regarding IBuilder._findBuildCandidate() please see
79 lib/lp/soyuz/tests/test_builder.py.
80
81 Start buildd-slave to be able to dispatch jobs.
82@@ -262,7 +265,7 @@
83
84 >>> bob_builder.virtualized = False
85 >>> flush_database_updates()
86- >>> bob_builder.dispatchBuildCandidate(ppa_job)
87+ >>> removeSecurityProxy(bob_builder)._dispatchBuildCandidate(ppa_job)
88 Traceback (most recent call last):
89 ...
90 AssertionError: Attempt to build non-virtual item on a virtual builder.
91@@ -273,7 +276,10 @@
92 >>> bob_builder.virtualized = True
93 >>> flush_database_updates()
94
95- >>> bob_builder.dispatchBuildCandidate(ppa_job)
96+ >>> dispatched_job = bob_builder.findAndStartJob()
97+ >>> ppa_job == dispatched_job
98+ True
99+
100 >>> flush_database_updates()
101
102 PPA job is building.
103@@ -328,7 +334,7 @@
104 implementation.
105
106 >>> BuilddSlaveTestSetup().setUp()
107- >>> bob_builder.dispatchBuildCandidate(sec_job)
108+ >>> removeSecurityProxy(bob_builder)._dispatchBuildCandidate(sec_job)
109 Traceback (most recent call last):
110 ...
111 AssertionError: Soyuz is not yet capable of building SECURITY uploads.
112@@ -336,7 +342,7 @@
113
114 To solve this problem temporarily until we start building security
115 uploads, we will mark builds targeted to the SECURITY pocket as
116-FAILEDTOBUILD during the findBuildCandidate look-up.
117+FAILEDTOBUILD during the _findBuildCandidate look-up.
118
119 We will also create another build candidate in breezy-autotest/i386 to
120 check if legitimate pending candidates will remain valid.
121@@ -360,7 +366,7 @@
122
123 >>> new_pub = old_pub.copyTo(
124 ... pending_build.distroseries, old_pub.pocket, pending_build.archive)
125- >>> candidate = bob_builder.findBuildCandidate()
126+ >>> candidate = removeSecurityProxy(bob_builder)._findBuildCandidate()
127 >>> flush_database_updates()
128 >>> candidate.id == pending_job.id
129 True
130
131=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
132--- lib/lp/soyuz/doc/buildd-slavescanner.txt 2010-01-05 16:30:29 +0000
133+++ lib/lp/soyuz/doc/buildd-slavescanner.txt 2010-01-11 21:56:15 +0000
134@@ -754,11 +754,8 @@
135
136 == Build Dispatching ==
137
138-Build dispatching can be entirely done via IBuilder content class via
139-the following API:
140-
141- * findCandidate: returns a suitable BuildQueue candidate
142- * dispatchBuildCandidate: dispatch a build for a given candidate.
143+Build dispatching can be entirely done via IBuilder content class
144+using the findAndStartJob method.
145
146 We will use SoyuzTestPublisher to simulate the required context in the
147 next tests. Let's initialise it.
148@@ -805,14 +802,14 @@
149 superseded source package releases in the queue and marks the
150 corresponding build record as SUPERSEDED.
151
152- >>> old_candidate = a_builder.findBuildCandidate()
153+ >>> old_candidate = removeSecurityProxy(a_builder)._findBuildCandidate()
154 >>> build = getUtility(IBuildSet).getByQueueEntry(old_candidate)
155 >>> print build.buildstate.name
156 NEEDSBUILD
157
158 The 'candidate' is constant until we dispatch it.
159
160- >>> new_candidate = a_builder.findBuildCandidate()
161+ >>> new_candidate = removeSecurityProxy(a_builder)._findBuildCandidate()
162 >>> new_candidate.id == old_candidate.id
163 True
164
165@@ -820,7 +817,7 @@
166 whether the candidate will still be found.
167
168 >>> build.archive.enabled = False
169- >>> new_candidate = a_builder.findBuildCandidate()
170+ >>> new_candidate = removeSecurityProxy(a_builder)._findBuildCandidate()
171 >>> new_candidate is None
172 True
173
174@@ -829,7 +826,7 @@
175 candidate will be found again.
176
177 >>> build.archive.enabled = True
178- >>> new_candidate = a_builder.findBuildCandidate()
179+ >>> new_candidate = removeSecurityProxy(a_builder)._findBuildCandidate()
180 >>> new_candidate.id == old_candidate.id
181 True
182
183@@ -852,7 +849,7 @@
184
185 Now, there we have another build candidate.
186
187- >>> new_candidate = a_builder.findBuildCandidate()
188+ >>> new_candidate = removeSecurityProxy(a_builder)._findBuildCandidate()
189 >>> new_candidate.id != old_candidate.id
190 True
191
192@@ -882,9 +879,10 @@
193
194 Let's try to find a new build candidate:
195
196- >>> another_candidate = a_builder.findBuildCandidate()
197+ >>> another_candidate = removeSecurityProxy(
198+ ... a_builder)._findBuildCandidate()
199
200-Since there are no more candidates at all, findBuildCandidate()
201+Since there are no more candidates at all, _findBuildCandidate()
202 returned None:
203
204 >>> print another_candidate
205@@ -897,7 +895,8 @@
206 >>> commit()
207 >>> LaunchpadZopelessLayer.switchDbUser(config.builddmaster.dbuser)
208
209- >>> another_candidate = a_builder.findBuildCandidate()
210+ >>> another_candidate = removeSecurityProxy(
211+ ... a_builder)._findBuildCandidate()
212 >>> another_candidate.id == new_candidate.id
213 True
214
215@@ -921,7 +920,8 @@
216 >>> print build.buildstate.name
217 NEEDSBUILD
218
219- >>> another_candidate = a_builder.findBuildCandidate()
220+ >>> another_candidate = removeSecurityProxy(
221+ ... a_builder)._findBuildCandidate()
222 >>> print another_candidate
223 None
224
225@@ -954,7 +954,7 @@
226 >>> a_builder.is_available
227 True
228 >>> candidate = a_build.createBuildQueueEntry()
229- >>> a_builder.dispatchBuildCandidate(candidate)
230+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate)
231 ensurepresent called, url=...
232 ensurepresent called,
233 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
234@@ -982,7 +982,7 @@
235 >>> a_builder.is_available
236 True
237 >>> candidate = a_build.createBuildQueueEntry()
238- >>> a_builder.dispatchBuildCandidate(candidate)
239+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate)
240 ensurepresent called, url=...
241 ensurepresent called,
242 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
243@@ -1034,7 +1034,7 @@
244 So, at moment, partner archive is still not relevant for builds in
245 hoary/i386. It's not passed to the builder.
246
247- >>> a_builder.dispatchBuildCandidate(candidate)
248+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate)
249 ensurepresent called, url=...
250 ensurepresent called,
251 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
252@@ -1084,7 +1084,8 @@
253 binary in hoary/i386, the partner archive gets included in the builder
254 sources_list.
255
256- >>> a_builder.dispatchBuildCandidate(partner_candidate)
257+ >>> removeSecurityProxy(
258+ ... a_builder)._dispatchBuildCandidate(partner_candidate)
259 ensurepresent called, url=...
260 ensurepresent called, url=http://localhost:58000/.../foo_666.dsc
261 OkSlave BUILDING
262@@ -1126,7 +1127,7 @@
263 >>> create_binary_publication_for(
264 ... cprov_archive, hoary, PackagePublishingStatus.PUBLISHED)
265
266- >>> a_builder.dispatchBuildCandidate(candidate)
267+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate)
268 ensurepresent called, url=...
269 ensurepresent called,
270 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
271@@ -1183,7 +1184,7 @@
272
273 This is so that the mangling tools will run over the built packages.
274
275- >>> a_builder.dispatchBuildCandidate(candidate)
276+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate)
277 ensurepresent called, url=...
278 ensurepresent called,
279 url=http://private-ppa.launchpad.dev/cprov/ppa/ubuntu/pool/main/m/mozilla-firefox/firefox_0.9.2.orig.tar.gz
280@@ -1223,7 +1224,7 @@
281 >>> LaunchpadZopelessLayer.switchDbUser(config.builddmaster.dbuser)
282 >>> login(ANONYMOUS)
283
284- >>> a_builder.dispatchBuildCandidate(candidate)
285+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate)
286 ensurepresent called, ...
287 ...
288 Ogre-component: main
289@@ -1302,7 +1303,7 @@
290 >>> setupBuildQueue(candidate, a_builder)
291 >>> last_stub_mail_count = len(stub.test_emails)
292
293- >>> a_builder.dispatchBuildCandidate(candidate)
294+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate)
295 ensurepresent called, url=...
296 ensurepresent called,
297 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
298@@ -1331,7 +1332,7 @@
299 >>> create_binary_publication_for(
300 ... mark_archive, hoary, PackagePublishingStatus.PUBLISHED)
301
302- >>> a_builder.dispatchBuildCandidate(candidate)
303+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate)
304 ensurepresent called, url=...
305 ensurepresent called,
306 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
307@@ -1376,7 +1377,7 @@
308
309 >>> hoary_i386.distroseries.status.name
310 'DEVELOPMENT'
311- >>> a_builder.dispatchBuildCandidate(updates_bqItem)
312+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(updates_bqItem)
313 Traceback (most recent call last):
314 ...
315 AssertionError: i386 build of evolution 1.0 in ubuntu hoary UPDATES (...) can not be built for pocket UPDATES: invalid pocket due to the series status of hoary.
316@@ -1401,7 +1402,7 @@
317 >>> removeSecurityProxy(build).pocket = (
318 ... PackagePublishingPocket.UPDATES)
319 >>> last_stub_mail_count = len(stub.test_emails)
320- >>> a_builder.dispatchBuildCandidate(bqItem3)
321+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(bqItem3)
322 ensurepresent called, url=...
323 ensurepresent called,
324 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
325@@ -1423,7 +1424,7 @@
326 >>> removeSecurityProxy(build).pocket = (
327 ... PackagePublishingPocket.PROPOSED)
328 >>> last_stub_mail_count = len(stub.test_emails)
329- >>> a_builder.dispatchBuildCandidate(bqItem3)
330+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(bqItem3)
331 ensurepresent called, url=...
332 ensurepresent called,
333 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
334@@ -1446,7 +1447,7 @@
335 >>> removeSecurityProxy(build).pocket = (
336 ... PackagePublishingPocket.BACKPORTS)
337 >>> last_stub_mail_count = len(stub.test_emails)
338- >>> a_builder.dispatchBuildCandidate(bqItem3)
339+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(bqItem3)
340 ensurepresent called, url=...
341 ensurepresent called,
342 url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz
343@@ -1477,13 +1478,13 @@
344 because Embargoed-Archives and Restricted-UI implementations are not
345 yet ready.
346
347- >>> a_builder.dispatchBuildCandidate(bqItem3)
348+ >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(bqItem3)
349 Traceback (most recent call last):
350 ...
351 AssertionError: Soyuz is not yet capable of building SECURITY uploads.
352
353-Builds for security pocket are marked as FAILEDTOBUILD inside
354-findBuildCandidate method, see doc/buildd-dispatching.txt
355+Builds for security pocket are marked as FAILEDTOBUILD inside the
356+_findBuildCandidate() method, see doc/buildd-dispatching.txt
357
358
359 == Builder Status Handler ==
360
361=== modified file 'lib/lp/soyuz/interfaces/builder.py'
362--- lib/lp/soyuz/interfaces/builder.py 2009-12-03 14:38:48 +0000
363+++ lib/lp/soyuz/interfaces/builder.py 2010-01-11 21:56:15 +0000
364@@ -242,24 +242,6 @@
365 :return: A librarian file alias.
366 """
367
368- def findBuildCandidate():
369- """Return the candidate for building.
370-
371- The pending BuildQueue item with the highest score for this builder
372- ProcessorFamily or None if no candidate is available.
373-
374- For public PPA builds, subsequent builds for a given ppa and
375- architecture will not be returned until the current build for
376- the ppa and architecture is finished.
377- """
378-
379- def dispatchBuildCandidate(candidate):
380- """Dispatch the given job to this builder.
381-
382- This method can only be executed in the builddmaster machine, since
383- it will actually issues the XMLRPC call to the buildd-slave.
384- """
385-
386 def handleTimeout(logger, error_message):
387 """Handle buildd slave communication timeout situations.
388
389@@ -274,6 +256,14 @@
390 :param error_message: The error message to be used for logging.
391 """
392
393+ def findAndStartJob(buildd_slave=None):
394+ """Find a job to run and send it to the buildd slave.
395+
396+ :param buildd_slave: An optional buildd slave that this builder should
397+ talk to.
398+ :return: the `IBuildQueue` instance found or None if no job was found.
399+ """
400+
401
402 class IBuilderSet(Interface):
403 """Collections of builders.
404
405=== modified file 'lib/lp/soyuz/model/builder.py'
406--- lib/lp/soyuz/model/builder.py 2009-12-24 06:57:25 +0000
407+++ lib/lp/soyuz/model/builder.py 2010-01-11 21:56:15 +0000
408@@ -386,9 +386,9 @@
409 return True
410
411 # XXX cprov 20071116: It should become part of the public
412- # findBuildCandidate once we start to detect superseded builds
413+ # _findBuildCandidate once we start to detect superseded builds
414 # at build creation time.
415- def _findBuildCandidate(self):
416+ def _findBinaryBuildCandidate(self):
417 """Return the highest priority build candidate for this builder.
418
419 Returns a pending IBuildQueue record queued for this builder
420@@ -487,10 +487,21 @@
421 logger = logging.getLogger('slave-scanner')
422 return logger
423
424- def findBuildCandidate(self):
425- """See `IBuilder`."""
426+ def _findBuildCandidate(self):
427+ """Find a candidate job for dispatch to an idle buildd slave.
428+
429+ The pending BuildQueue item with the highest score for this builder
430+ ProcessorFamily or None if no candidate is available.
431+
432+ For public PPA builds, subsequent builds for a given ppa and
433+ architecture will not be returned until the current build for
434+ the ppa and architecture is finished.
435+
436+ :return: A binary build candidate job.
437+ """
438+
439 logger = self._getSlaveScannerLogger()
440- candidate = self._findBuildCandidate()
441+ candidate = self._findBinaryBuildCandidate()
442
443 # Mark build records targeted to old source versions as SUPERSEDED
444 # and build records target to SECURITY pocket as FAILEDTOBUILD.
445@@ -507,7 +518,7 @@
446 % (build.id, candidate.id))
447 build.buildstate = BuildStatus.FAILEDTOBUILD
448 candidate.destroySelf()
449- candidate = self._findBuildCandidate()
450+ candidate = self._findBinaryBuildCandidate()
451 continue
452
453 publication = build.current_source_publication
454@@ -520,7 +531,7 @@
455 % (build.id, candidate.id))
456 build.buildstate = BuildStatus.SUPERSEDED
457 candidate.destroySelf()
458- candidate = self._findBuildCandidate()
459+ candidate = self._findBinaryBuildCandidate()
460 continue
461
462 return candidate
463@@ -528,8 +539,14 @@
464 # No candidate was found.
465 return None
466
467- def dispatchBuildCandidate(self, candidate):
468- """See `IBuilder`."""
469+ def _dispatchBuildCandidate(self, candidate):
470+ """Dispatch the pending job to the associated buildd slave.
471+
472+ This method can only be executed in the builddmaster machine, since
473+ it will actually issues the XMLRPC call to the buildd-slave.
474+
475+ :param candidate: The job to dispatch.
476+ """
477 logger = self._getSlaveScannerLogger()
478 try:
479 self.startBuild(candidate, logger)
480@@ -563,6 +580,21 @@
481 exc_info=True)
482 self.failbuilder(error_message)
483
484+ def findAndStartJob(self, buildd_slave=None):
485+ """See IBuilder."""
486+ logger = self._getSlaveScannerLogger()
487+ candidate = self._findBuildCandidate()
488+
489+ if candidate is None:
490+ logger.debug("No build candidates available for builder.")
491+ return None
492+
493+ if buildd_slave is not None:
494+ self.setSlaveForTesting(buildd_slave)
495+
496+ self._dispatchBuildCandidate(candidate)
497+ return candidate
498+
499
500 class BuilderSet(object):
501 """See IBuilderSet"""
502
503=== modified file 'lib/lp/soyuz/scripts/buildd.py'
504--- lib/lp/soyuz/scripts/buildd.py 2009-10-26 18:40:04 +0000
505+++ lib/lp/soyuz/scripts/buildd.py 2010-01-11 21:56:15 +0000
506@@ -201,12 +201,10 @@
507 if not builder.is_available:
508 self.logger.warn('builder is not available. Ignored.')
509 continue
510- candidate = builder.findBuildCandidate()
511+
512+ candidate = builder.findAndStartJob()
513 if candidate is None:
514- self.logger.debug(
515- "No candidates available for builder.")
516 continue
517- builder.dispatchBuildCandidate(candidate)
518 self.txn.commit()
519
520 self.logger.info("Slave Scan Process Finished.")
521
522=== modified file 'lib/lp/soyuz/tests/test_builder.py'
523--- lib/lp/soyuz/tests/test_builder.py 2009-12-02 15:18:46 +0000
524+++ lib/lp/soyuz/tests/test_builder.py 2010-01-11 21:56:15 +0000
525@@ -6,10 +6,11 @@
526 import unittest
527
528 from zope.component import getUtility
529+from zope.security.proxy import removeSecurityProxy
530
531 from canonical.testing import LaunchpadZopelessLayer
532 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
533- BuildBehaviorMismatch, IBuildFarmJobBehavior)
534+ IBuildFarmJobBehavior)
535 from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior
536 from lp.soyuz.interfaces.archive import ArchivePurpose
537 from lp.soyuz.interfaces.build import BuildStatus, IBuildSet
538@@ -34,13 +35,13 @@
539 # Create some i386 builders ready to build PPA builds. Two
540 # already exist in sampledata so we'll use those first.
541 self.builder1 = getUtility(IBuilderSet)['bob']
542- self.builder2 = getUtility(IBuilderSet)['frog']
543+ self.frog_builder = getUtility(IBuilderSet)['frog']
544 self.builder3 = self.factory.makeBuilder(name='builder3')
545 self.builder4 = self.factory.makeBuilder(name='builder4')
546 self.builder5 = self.factory.makeBuilder(name='builder5')
547 self.builders = [
548 self.builder1,
549- self.builder2,
550+ self.frog_builder,
551 self.builder3,
552 self.builder4,
553 self.builder5,
554@@ -81,7 +82,8 @@
555 # there's only one builder available.
556
557 # Asking frog to find a candidate should give us the joesppa build.
558- next_job = self.frog_builder.findBuildCandidate()
559+ next_job = removeSecurityProxy(
560+ self.frog_builder)._findBuildCandidate()
561 build = getUtility(IBuildSet).getByQueueEntry(next_job)
562 self.assertEqual('joesppa', build.archive.name)
563
564@@ -89,7 +91,8 @@
565 # returned.
566 self.bob_builder.builderok = False
567 self.bob_builder.manual = False
568- next_job = self.frog_builder.findBuildCandidate()
569+ next_job = removeSecurityProxy(
570+ self.frog_builder)._findBuildCandidate()
571 build = getUtility(IBuildSet).getByQueueEntry(next_job)
572 self.assertEqual('joesppa', build.archive.name)
573
574@@ -162,7 +165,7 @@
575 def test_findBuildCandidate_first_build_started(self):
576 # A PPA cannot start a build if it would use 80% or more of the
577 # builders.
578- next_job = self.builder4.findBuildCandidate()
579+ next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
580 build = getUtility(IBuildSet).getByQueueEntry(next_job)
581 self.failIfEqual('joesppa', build.archive.name)
582
583@@ -170,7 +173,7 @@
584 # When joe's first ppa build finishes, his fourth i386 build
585 # will be the next build candidate.
586 self.joe_builds[0].buildstate = BuildStatus.FAILEDTOBUILD
587- next_job = self.builder4.findBuildCandidate()
588+ next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
589 build = getUtility(IBuildSet).getByQueueEntry(next_job)
590 self.failUnlessEqual('joesppa', build.archive.name)
591
592@@ -179,7 +182,7 @@
593 # for the one architecture.
594 self.ppa_joe.private = True
595 self.ppa_joe.buildd_secret = 'sekrit'
596- next_job = self.builder4.findBuildCandidate()
597+ next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
598 build = getUtility(IBuildSet).getByQueueEntry(next_job)
599 self.failUnlessEqual('joesppa', build.archive.name)
600
601@@ -205,7 +208,8 @@
602 # Normal archives are not restricted to serial builds per
603 # arch.
604
605- next_job = self.builder2.findBuildCandidate()
606+ next_job = removeSecurityProxy(
607+ self.frog_builder)._findBuildCandidate()
608 build = getUtility(IBuildSet).getByQueueEntry(next_job)
609 self.failUnlessEqual('primary', build.archive.name)
610 self.failUnlessEqual('gedit', build.sourcepackagerelease.name)
611@@ -213,8 +217,9 @@
612 # Now even if we set the build building, we'll still get the
613 # second non-ppa build for the same archive as the next candidate.
614 build.buildstate = BuildStatus.BUILDING
615- build.builder = self.builder2
616- next_job = self.builder2.findBuildCandidate()
617+ build.builder = self.frog_builder
618+ next_job = removeSecurityProxy(
619+ self.frog_builder)._findBuildCandidate()
620 build = getUtility(IBuildSet).getByQueueEntry(next_job)
621 self.failUnlessEqual('primary', build.archive.name)
622 self.failUnlessEqual('firefox', build.sourcepackagerelease.name)