Merge lp:~al-maisan/launchpad/ibuilder-api-cleanup-505647 into lp:launchpad
- ibuilder-api-cleanup-505647
- Merge into devel
Proposed by
Muharem Hrnjadovic
| 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 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Björn Tillenius (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
| Muharem Hrnjadovic (al-maisan) wrote : | # |
Revision history for this message
| 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/
> --- lib/lp/
> +++ lib/lp/
> @@ -340,17 +340,9 @@
> transaction.
> continue
>
> - candidate = builder.
> - if candidate is None:
> - self.logger.debug(
> - "No build candidates available for builder.")
> - continue
> -
> slave = RecordingSlave(
> - builder.
> -
> - builder.
> - if builder.currentjob is not None:
> + candidate = builder.
> + if candidate is not None:
> recording_
> transaction.
Here you changed it to commit when no candidates are found...
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -201,12 +201,10 @@
> if not builder.
> self.logger.
> continue
> - candidate = builder.
> +
> + candidate = builder.
> if candidate is None:
> - self.logger.debug(
> - "No candidates available for builder.")
> continue
> - builder.
> 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/
> --- lib/lp/
> +++ lib/lp/
> @@ -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(
> - self.builder2 = getUtility(
> + self.frog_builder = removeSecurityP
> + getUtility(
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.
> - self.builder4 = self.factory.
> + self.builder4 = removeSecurityP
> + self.factory.
Same comment here.
> self.builder5 = self.factory.
> self.b...
review:
Needs Information
Revision history for this message
| 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/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -340,17 +340,9 @@
>> transaction.
>> continue
>>
>> - candidate = builder.
>> - if candidate is None:
>> - self.logger.debug(
>> - "No build candidates available for builder.")
>> - continue
>> -
>> slave = RecordingSlave(
>> - builder.
>> -
>> - builder.
>> - if builder.currentjob is not None:
>> + candidate = builder.
>> + if candidate is not None:
>> recording_
>> transaction.
>
> 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/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -201,12 +201,10 @@
>> if not builder.
>> self.logger.
>> continue
>> - candidate = builder.
>> +
>> + candidate = builder.
>> if candidate is None:
>> - self.logger.debug(
>> - "No candidates available for builder.")
>> continue
>> - builder.
>> 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/
>> --- lib/lp/
>> +++ lib/lp/
>
>> @@ -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(
>> - self.builder2 = getUtility(
>> + self.frog_builder = removeSecurityP
>> + getUtility(
>
> 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.
>> - ...
| 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 | 344 | candidate = builder.findAndStartJob(buildd_slave=slave) | 344 | candidate = builder.findAndStartJob(buildd_slave=slave) |
| 6 | 345 | if candidate is not None: | 345 | if candidate is not None: |
| 7 | 346 | recording_slaves.append(slave) | 346 | recording_slaves.append(slave) |
| 9 | 347 | transaction.commit() | 347 | transaction.commit() |
| 10 | 348 | 348 | ||
| 11 | 349 | return recording_slaves | 349 | return recording_slaves |
| 12 | 350 | 350 | ||
| 13 | 351 | 351 | ||
| 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 | 35 | # Create some i386 builders ready to build PPA builds. Two | 35 | # Create some i386 builders ready to build PPA builds. Two |
| 19 | 36 | # already exist in sampledata so we'll use those first. | 36 | # already exist in sampledata so we'll use those first. |
| 20 | 37 | self.builder1 = getUtility(IBuilderSet)['bob'] | 37 | self.builder1 = getUtility(IBuilderSet)['bob'] |
| 23 | 38 | self.frog_builder = removeSecurityProxy( | 38 | self.frog_builder = getUtility(IBuilderSet)['frog'] |
| 22 | 39 | getUtility(IBuilderSet)['frog']) | ||
| 24 | 40 | self.builder3 = self.factory.makeBuilder(name='builder3') | 39 | self.builder3 = self.factory.makeBuilder(name='builder3') |
| 27 | 41 | self.builder4 = removeSecurityProxy( | 40 | self.builder4 = self.factory.makeBuilder(name='builder4') |
| 26 | 42 | self.factory.makeBuilder(name='builder4')) | ||
| 28 | 43 | self.builder5 = self.factory.makeBuilder(name='builder5') | 41 | self.builder5 = self.factory.makeBuilder(name='builder5') |
| 29 | 44 | self.builders = [ | 42 | self.builders = [ |
| 30 | 45 | self.builder1, | 43 | self.builder1, |
| 31 | @@ -65,8 +63,7 @@ | |||
| 32 | 65 | self.publisher.prepareBreezyAutotest() | 63 | self.publisher.prepareBreezyAutotest() |
| 33 | 66 | 64 | ||
| 34 | 67 | self.bob_builder = getUtility(IBuilderSet)['bob'] | 65 | self.bob_builder = getUtility(IBuilderSet)['bob'] |
| 37 | 68 | self.frog_builder = removeSecurityProxy( | 66 | self.frog_builder = getUtility(IBuilderSet)['frog'] |
| 36 | 69 | getUtility(IBuilderSet)['frog']) | ||
| 38 | 70 | 67 | ||
| 39 | 71 | # Disable bob so only frog is available. | 68 | # Disable bob so only frog is available. |
| 40 | 72 | self.bob_builder.manual = True | 69 | self.bob_builder.manual = True |
| 41 | @@ -85,7 +82,8 @@ | |||
| 42 | 85 | # there's only one builder available. | 82 | # there's only one builder available. |
| 43 | 86 | 83 | ||
| 44 | 87 | # Asking frog to find a candidate should give us the joesppa build. | 84 | # Asking frog to find a candidate should give us the joesppa build. |
| 46 | 88 | next_job = self.frog_builder._findBuildCandidate() | 85 | next_job = removeSecurityProxy( |
| 47 | 86 | self.frog_builder)._findBuildCandidate() | ||
| 48 | 89 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 87 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 49 | 90 | self.assertEqual('joesppa', build.archive.name) | 88 | self.assertEqual('joesppa', build.archive.name) |
| 50 | 91 | 89 | ||
| 51 | @@ -93,7 +91,8 @@ | |||
| 52 | 93 | # returned. | 91 | # returned. |
| 53 | 94 | self.bob_builder.builderok = False | 92 | self.bob_builder.builderok = False |
| 54 | 95 | self.bob_builder.manual = False | 93 | self.bob_builder.manual = False |
| 56 | 96 | next_job = self.frog_builder._findBuildCandidate() | 94 | next_job = removeSecurityProxy( |
| 57 | 95 | self.frog_builder)._findBuildCandidate() | ||
| 58 | 97 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 96 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 59 | 98 | self.assertEqual('joesppa', build.archive.name) | 97 | self.assertEqual('joesppa', build.archive.name) |
| 60 | 99 | 98 | ||
| 61 | @@ -166,7 +165,7 @@ | |||
| 62 | 166 | def test_findBuildCandidate_first_build_started(self): | 165 | def test_findBuildCandidate_first_build_started(self): |
| 63 | 167 | # A PPA cannot start a build if it would use 80% or more of the | 166 | # A PPA cannot start a build if it would use 80% or more of the |
| 64 | 168 | # builders. | 167 | # builders. |
| 66 | 169 | next_job = self.builder4._findBuildCandidate() | 168 | next_job = removeSecurityProxy(self.builder4)._findBuildCandidate() |
| 67 | 170 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 169 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 68 | 171 | self.failIfEqual('joesppa', build.archive.name) | 170 | self.failIfEqual('joesppa', build.archive.name) |
| 69 | 172 | 171 | ||
| 70 | @@ -174,7 +173,7 @@ | |||
| 71 | 174 | # When joe's first ppa build finishes, his fourth i386 build | 173 | # When joe's first ppa build finishes, his fourth i386 build |
| 72 | 175 | # will be the next build candidate. | 174 | # will be the next build candidate. |
| 73 | 176 | self.joe_builds[0].buildstate = BuildStatus.FAILEDTOBUILD | 175 | self.joe_builds[0].buildstate = BuildStatus.FAILEDTOBUILD |
| 75 | 177 | next_job = self.builder4._findBuildCandidate() | 176 | next_job = removeSecurityProxy(self.builder4)._findBuildCandidate() |
| 76 | 178 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 177 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 77 | 179 | self.failUnlessEqual('joesppa', build.archive.name) | 178 | self.failUnlessEqual('joesppa', build.archive.name) |
| 78 | 180 | 179 | ||
| 79 | @@ -183,7 +182,7 @@ | |||
| 80 | 183 | # for the one architecture. | 182 | # for the one architecture. |
| 81 | 184 | self.ppa_joe.private = True | 183 | self.ppa_joe.private = True |
| 82 | 185 | self.ppa_joe.buildd_secret = 'sekrit' | 184 | self.ppa_joe.buildd_secret = 'sekrit' |
| 84 | 186 | next_job = self.builder4._findBuildCandidate() | 185 | next_job = removeSecurityProxy(self.builder4)._findBuildCandidate() |
| 85 | 187 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 186 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 86 | 188 | self.failUnlessEqual('joesppa', build.archive.name) | 187 | self.failUnlessEqual('joesppa', build.archive.name) |
| 87 | 189 | 188 | ||
| 88 | @@ -209,7 +208,8 @@ | |||
| 89 | 209 | # Normal archives are not restricted to serial builds per | 208 | # Normal archives are not restricted to serial builds per |
| 90 | 210 | # arch. | 209 | # arch. |
| 91 | 211 | 210 | ||
| 93 | 212 | next_job = self.frog_builder._findBuildCandidate() | 211 | next_job = removeSecurityProxy( |
| 94 | 212 | self.frog_builder)._findBuildCandidate() | ||
| 95 | 213 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 213 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 96 | 214 | self.failUnlessEqual('primary', build.archive.name) | 214 | self.failUnlessEqual('primary', build.archive.name) |
| 97 | 215 | self.failUnlessEqual('gedit', build.sourcepackagerelease.name) | 215 | self.failUnlessEqual('gedit', build.sourcepackagerelease.name) |
| 98 | @@ -218,7 +218,8 @@ | |||
| 99 | 218 | # second non-ppa build for the same archive as the next candidate. | 218 | # second non-ppa build for the same archive as the next candidate. |
| 100 | 219 | build.buildstate = BuildStatus.BUILDING | 219 | build.buildstate = BuildStatus.BUILDING |
| 101 | 220 | build.builder = self.frog_builder | 220 | build.builder = self.frog_builder |
| 103 | 221 | next_job = self.frog_builder._findBuildCandidate() | 221 | next_job = removeSecurityProxy( |
| 104 | 222 | self.frog_builder)._findBuildCandidate() | ||
| 105 | 222 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 223 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 106 | 223 | self.failUnlessEqual('primary', build.archive.name) | 224 | self.failUnlessEqual('primary', build.archive.name) |
| 107 | 224 | self.failUnlessEqual('firefox', build.sourcepackagerelease.name) | 225 | self.failUnlessEqual('firefox', build.sourcepackagerelease.name) |
Revision history for this message
| 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 | 340 | transaction.commit() | 340 | transaction.commit() |
| 6 | 341 | continue | 341 | continue |
| 7 | 342 | 342 | ||
| 8 | 343 | candidate = builder.findBuildCandidate() | ||
| 9 | 344 | if candidate is None: | ||
| 10 | 345 | self.logger.debug( | ||
| 11 | 346 | "No build candidates available for builder.") | ||
| 12 | 347 | continue | ||
| 13 | 348 | |||
| 14 | 349 | slave = RecordingSlave(builder.name, builder.url, builder.vm_host) | 343 | slave = RecordingSlave(builder.name, builder.url, builder.vm_host) |
| 18 | 350 | builder.setSlaveForTesting(slave) | 344 | candidate = builder.findAndStartJob(buildd_slave=slave) |
| 16 | 351 | |||
| 17 | 352 | builder.dispatchBuildCandidate(candidate) | ||
| 19 | 353 | if builder.currentjob is not None: | 345 | if builder.currentjob is not None: |
| 20 | 354 | recording_slaves.append(slave) | 346 | recording_slaves.append(slave) |
| 22 | 355 | transaction.commit() | 347 | transaction.commit() |
| 23 | 356 | 348 | ||
| 24 | 357 | return recording_slaves | 349 | return recording_slaves |
| 25 | 358 | 350 | ||
| 26 | 359 | 351 | ||
| 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 | 128 | Now let's check the build candidates which will be considered for the | 128 | Now let's check the build candidates which will be considered for the |
| 32 | 129 | builder 'bob': | 129 | builder 'bob': |
| 33 | 130 | 130 | ||
| 35 | 131 | >>> job = bob_builder.findBuildCandidate() | 131 | >>> from zope.security.proxy import removeSecurityProxy |
| 36 | 132 | >>> job = removeSecurityProxy(bob_builder)._findBuildCandidate() | ||
| 37 | 132 | 133 | ||
| 38 | 133 | The single BuildQueue found is a non-virtual pending build: | 134 | The single BuildQueue found is a non-virtual pending build: |
| 39 | 134 | 135 | ||
| 40 | @@ -157,14 +158,16 @@ | |||
| 41 | 157 | ... 'foo.dsc', len(content), StringIO(content), 'application/dsc') | 158 | ... 'foo.dsc', len(content), StringIO(content), 'application/dsc') |
| 42 | 158 | 159 | ||
| 43 | 159 | >>> sprf = build.sourcepackagerelease.files[0] | 160 | >>> sprf = build.sourcepackagerelease.files[0] |
| 44 | 160 | >>> from zope.security.proxy import removeSecurityProxy | ||
| 45 | 161 | >>> naked_sprf = removeSecurityProxy(sprf) | 161 | >>> naked_sprf = removeSecurityProxy(sprf) |
| 46 | 162 | >>> naked_sprf.libraryfile = getUtility(ILibraryFileAliasSet)[alias_id] | 162 | >>> naked_sprf.libraryfile = getUtility(ILibraryFileAliasSet)[alias_id] |
| 47 | 163 | >>> flush_database_updates() | 163 | >>> flush_database_updates() |
| 48 | 164 | 164 | ||
| 49 | 165 | Check the dispatching method itself: | 165 | Check the dispatching method itself: |
| 50 | 166 | 166 | ||
| 52 | 167 | >>> bob_builder.dispatchBuildCandidate(job) | 167 | >>> dispatched_job = bob_builder.findAndStartJob() |
| 53 | 168 | >>> job == dispatched_job | ||
| 54 | 169 | True | ||
| 55 | 170 | |||
| 56 | 168 | >>> flush_database_updates() | 171 | >>> flush_database_updates() |
| 57 | 169 | 172 | ||
| 58 | 170 | Verify if the job (BuildQueue) was updated appropriately: | 173 | Verify if the job (BuildQueue) was updated appropriately: |
| 59 | @@ -224,7 +227,7 @@ | |||
| 60 | 224 | >>> bob_builder.vm_host = 'localhost.ppa' | 227 | >>> bob_builder.vm_host = 'localhost.ppa' |
| 61 | 225 | >>> syncUpdate(bob_builder) | 228 | >>> syncUpdate(bob_builder) |
| 62 | 226 | 229 | ||
| 64 | 227 | >>> job = bob_builder.findBuildCandidate() | 230 | >>> job = removeSecurityProxy(bob_builder)._findBuildCandidate() |
| 65 | 228 | >>> print job | 231 | >>> print job |
| 66 | 229 | None | 232 | None |
| 67 | 230 | 233 | ||
| 68 | @@ -245,11 +248,11 @@ | |||
| 69 | 245 | >>> bob_builder.virtualized = True | 248 | >>> bob_builder.virtualized = True |
| 70 | 246 | >>> syncUpdate(bob_builder) | 249 | >>> syncUpdate(bob_builder) |
| 71 | 247 | 250 | ||
| 73 | 248 | >>> job = bob_builder.findBuildCandidate() | 251 | >>> job = removeSecurityProxy(bob_builder)._findBuildCandidate() |
| 74 | 249 | >>> ppa_job.id == job.id | 252 | >>> ppa_job.id == job.id |
| 75 | 250 | True | 253 | True |
| 76 | 251 | 254 | ||
| 78 | 252 | For further details regarding IBuilder.findBuildCandidate() please see | 255 | For further details regarding IBuilder._findBuildCandidate() please see |
| 79 | 253 | lib/lp/soyuz/tests/test_builder.py. | 256 | lib/lp/soyuz/tests/test_builder.py. |
| 80 | 254 | 257 | ||
| 81 | 255 | Start buildd-slave to be able to dispatch jobs. | 258 | Start buildd-slave to be able to dispatch jobs. |
| 82 | @@ -262,7 +265,7 @@ | |||
| 83 | 262 | 265 | ||
| 84 | 263 | >>> bob_builder.virtualized = False | 266 | >>> bob_builder.virtualized = False |
| 85 | 264 | >>> flush_database_updates() | 267 | >>> flush_database_updates() |
| 87 | 265 | >>> bob_builder.dispatchBuildCandidate(ppa_job) | 268 | >>> removeSecurityProxy(bob_builder)._dispatchBuildCandidate(ppa_job) |
| 88 | 266 | Traceback (most recent call last): | 269 | Traceback (most recent call last): |
| 89 | 267 | ... | 270 | ... |
| 90 | 268 | AssertionError: Attempt to build non-virtual item on a virtual builder. | 271 | AssertionError: Attempt to build non-virtual item on a virtual builder. |
| 91 | @@ -273,7 +276,10 @@ | |||
| 92 | 273 | >>> bob_builder.virtualized = True | 276 | >>> bob_builder.virtualized = True |
| 93 | 274 | >>> flush_database_updates() | 277 | >>> flush_database_updates() |
| 94 | 275 | 278 | ||
| 96 | 276 | >>> bob_builder.dispatchBuildCandidate(ppa_job) | 279 | >>> dispatched_job = bob_builder.findAndStartJob() |
| 97 | 280 | >>> ppa_job == dispatched_job | ||
| 98 | 281 | True | ||
| 99 | 282 | |||
| 100 | 277 | >>> flush_database_updates() | 283 | >>> flush_database_updates() |
| 101 | 278 | 284 | ||
| 102 | 279 | PPA job is building. | 285 | PPA job is building. |
| 103 | @@ -328,7 +334,7 @@ | |||
| 104 | 328 | implementation. | 334 | implementation. |
| 105 | 329 | 335 | ||
| 106 | 330 | >>> BuilddSlaveTestSetup().setUp() | 336 | >>> BuilddSlaveTestSetup().setUp() |
| 108 | 331 | >>> bob_builder.dispatchBuildCandidate(sec_job) | 337 | >>> removeSecurityProxy(bob_builder)._dispatchBuildCandidate(sec_job) |
| 109 | 332 | Traceback (most recent call last): | 338 | Traceback (most recent call last): |
| 110 | 333 | ... | 339 | ... |
| 111 | 334 | AssertionError: Soyuz is not yet capable of building SECURITY uploads. | 340 | AssertionError: Soyuz is not yet capable of building SECURITY uploads. |
| 112 | @@ -336,7 +342,7 @@ | |||
| 113 | 336 | 342 | ||
| 114 | 337 | To solve this problem temporarily until we start building security | 343 | To solve this problem temporarily until we start building security |
| 115 | 338 | uploads, we will mark builds targeted to the SECURITY pocket as | 344 | uploads, we will mark builds targeted to the SECURITY pocket as |
| 117 | 339 | FAILEDTOBUILD during the findBuildCandidate look-up. | 345 | FAILEDTOBUILD during the _findBuildCandidate look-up. |
| 118 | 340 | 346 | ||
| 119 | 341 | We will also create another build candidate in breezy-autotest/i386 to | 347 | We will also create another build candidate in breezy-autotest/i386 to |
| 120 | 342 | check if legitimate pending candidates will remain valid. | 348 | check if legitimate pending candidates will remain valid. |
| 121 | @@ -360,7 +366,7 @@ | |||
| 122 | 360 | 366 | ||
| 123 | 361 | >>> new_pub = old_pub.copyTo( | 367 | >>> new_pub = old_pub.copyTo( |
| 124 | 362 | ... pending_build.distroseries, old_pub.pocket, pending_build.archive) | 368 | ... pending_build.distroseries, old_pub.pocket, pending_build.archive) |
| 126 | 363 | >>> candidate = bob_builder.findBuildCandidate() | 369 | >>> candidate = removeSecurityProxy(bob_builder)._findBuildCandidate() |
| 127 | 364 | >>> flush_database_updates() | 370 | >>> flush_database_updates() |
| 128 | 365 | >>> candidate.id == pending_job.id | 371 | >>> candidate.id == pending_job.id |
| 129 | 366 | True | 372 | True |
| 130 | 367 | 373 | ||
| 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 | 754 | 754 | ||
| 136 | 755 | == Build Dispatching == | 755 | == Build Dispatching == |
| 137 | 756 | 756 | ||
| 143 | 757 | Build dispatching can be entirely done via IBuilder content class via | 757 | Build dispatching can be entirely done via IBuilder content class |
| 144 | 758 | the following API: | 758 | using the findAndStartJob method. |
| 140 | 759 | |||
| 141 | 760 | * findCandidate: returns a suitable BuildQueue candidate | ||
| 142 | 761 | * dispatchBuildCandidate: dispatch a build for a given candidate. | ||
| 145 | 762 | 759 | ||
| 146 | 763 | We will use SoyuzTestPublisher to simulate the required context in the | 760 | We will use SoyuzTestPublisher to simulate the required context in the |
| 147 | 764 | next tests. Let's initialise it. | 761 | next tests. Let's initialise it. |
| 148 | @@ -805,14 +802,14 @@ | |||
| 149 | 805 | superseded source package releases in the queue and marks the | 802 | superseded source package releases in the queue and marks the |
| 150 | 806 | corresponding build record as SUPERSEDED. | 803 | corresponding build record as SUPERSEDED. |
| 151 | 807 | 804 | ||
| 153 | 808 | >>> old_candidate = a_builder.findBuildCandidate() | 805 | >>> old_candidate = removeSecurityProxy(a_builder)._findBuildCandidate() |
| 154 | 809 | >>> build = getUtility(IBuildSet).getByQueueEntry(old_candidate) | 806 | >>> build = getUtility(IBuildSet).getByQueueEntry(old_candidate) |
| 155 | 810 | >>> print build.buildstate.name | 807 | >>> print build.buildstate.name |
| 156 | 811 | NEEDSBUILD | 808 | NEEDSBUILD |
| 157 | 812 | 809 | ||
| 158 | 813 | The 'candidate' is constant until we dispatch it. | 810 | The 'candidate' is constant until we dispatch it. |
| 159 | 814 | 811 | ||
| 161 | 815 | >>> new_candidate = a_builder.findBuildCandidate() | 812 | >>> new_candidate = removeSecurityProxy(a_builder)._findBuildCandidate() |
| 162 | 816 | >>> new_candidate.id == old_candidate.id | 813 | >>> new_candidate.id == old_candidate.id |
| 163 | 817 | True | 814 | True |
| 164 | 818 | 815 | ||
| 165 | @@ -820,7 +817,7 @@ | |||
| 166 | 820 | whether the candidate will still be found. | 817 | whether the candidate will still be found. |
| 167 | 821 | 818 | ||
| 168 | 822 | >>> build.archive.enabled = False | 819 | >>> build.archive.enabled = False |
| 170 | 823 | >>> new_candidate = a_builder.findBuildCandidate() | 820 | >>> new_candidate = removeSecurityProxy(a_builder)._findBuildCandidate() |
| 171 | 824 | >>> new_candidate is None | 821 | >>> new_candidate is None |
| 172 | 825 | True | 822 | True |
| 173 | 826 | 823 | ||
| 174 | @@ -829,7 +826,7 @@ | |||
| 175 | 829 | candidate will be found again. | 826 | candidate will be found again. |
| 176 | 830 | 827 | ||
| 177 | 831 | >>> build.archive.enabled = True | 828 | >>> build.archive.enabled = True |
| 179 | 832 | >>> new_candidate = a_builder.findBuildCandidate() | 829 | >>> new_candidate = removeSecurityProxy(a_builder)._findBuildCandidate() |
| 180 | 833 | >>> new_candidate.id == old_candidate.id | 830 | >>> new_candidate.id == old_candidate.id |
| 181 | 834 | True | 831 | True |
| 182 | 835 | 832 | ||
| 183 | @@ -852,7 +849,7 @@ | |||
| 184 | 852 | 849 | ||
| 185 | 853 | Now, there we have another build candidate. | 850 | Now, there we have another build candidate. |
| 186 | 854 | 851 | ||
| 188 | 855 | >>> new_candidate = a_builder.findBuildCandidate() | 852 | >>> new_candidate = removeSecurityProxy(a_builder)._findBuildCandidate() |
| 189 | 856 | >>> new_candidate.id != old_candidate.id | 853 | >>> new_candidate.id != old_candidate.id |
| 190 | 857 | True | 854 | True |
| 191 | 858 | 855 | ||
| 192 | @@ -882,9 +879,10 @@ | |||
| 193 | 882 | 879 | ||
| 194 | 883 | Let's try to find a new build candidate: | 880 | Let's try to find a new build candidate: |
| 195 | 884 | 881 | ||
| 197 | 885 | >>> another_candidate = a_builder.findBuildCandidate() | 882 | >>> another_candidate = removeSecurityProxy( |
| 198 | 883 | ... a_builder)._findBuildCandidate() | ||
| 199 | 886 | 884 | ||
| 201 | 887 | Since there are no more candidates at all, findBuildCandidate() | 885 | Since there are no more candidates at all, _findBuildCandidate() |
| 202 | 888 | returned None: | 886 | returned None: |
| 203 | 889 | 887 | ||
| 204 | 890 | >>> print another_candidate | 888 | >>> print another_candidate |
| 205 | @@ -897,7 +895,8 @@ | |||
| 206 | 897 | >>> commit() | 895 | >>> commit() |
| 207 | 898 | >>> LaunchpadZopelessLayer.switchDbUser(config.builddmaster.dbuser) | 896 | >>> LaunchpadZopelessLayer.switchDbUser(config.builddmaster.dbuser) |
| 208 | 899 | 897 | ||
| 210 | 900 | >>> another_candidate = a_builder.findBuildCandidate() | 898 | >>> another_candidate = removeSecurityProxy( |
| 211 | 899 | ... a_builder)._findBuildCandidate() | ||
| 212 | 901 | >>> another_candidate.id == new_candidate.id | 900 | >>> another_candidate.id == new_candidate.id |
| 213 | 902 | True | 901 | True |
| 214 | 903 | 902 | ||
| 215 | @@ -921,7 +920,8 @@ | |||
| 216 | 921 | >>> print build.buildstate.name | 920 | >>> print build.buildstate.name |
| 217 | 922 | NEEDSBUILD | 921 | NEEDSBUILD |
| 218 | 923 | 922 | ||
| 220 | 924 | >>> another_candidate = a_builder.findBuildCandidate() | 923 | >>> another_candidate = removeSecurityProxy( |
| 221 | 924 | ... a_builder)._findBuildCandidate() | ||
| 222 | 925 | >>> print another_candidate | 925 | >>> print another_candidate |
| 223 | 926 | None | 926 | None |
| 224 | 927 | 927 | ||
| 225 | @@ -954,7 +954,7 @@ | |||
| 226 | 954 | >>> a_builder.is_available | 954 | >>> a_builder.is_available |
| 227 | 955 | True | 955 | True |
| 228 | 956 | >>> candidate = a_build.createBuildQueueEntry() | 956 | >>> candidate = a_build.createBuildQueueEntry() |
| 230 | 957 | >>> a_builder.dispatchBuildCandidate(candidate) | 957 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate) |
| 231 | 958 | ensurepresent called, url=... | 958 | ensurepresent called, url=... |
| 232 | 959 | ensurepresent called, | 959 | ensurepresent called, |
| 233 | 960 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 960 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 234 | @@ -982,7 +982,7 @@ | |||
| 235 | 982 | >>> a_builder.is_available | 982 | >>> a_builder.is_available |
| 236 | 983 | True | 983 | True |
| 237 | 984 | >>> candidate = a_build.createBuildQueueEntry() | 984 | >>> candidate = a_build.createBuildQueueEntry() |
| 239 | 985 | >>> a_builder.dispatchBuildCandidate(candidate) | 985 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate) |
| 240 | 986 | ensurepresent called, url=... | 986 | ensurepresent called, url=... |
| 241 | 987 | ensurepresent called, | 987 | ensurepresent called, |
| 242 | 988 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 988 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 243 | @@ -1034,7 +1034,7 @@ | |||
| 244 | 1034 | So, at moment, partner archive is still not relevant for builds in | 1034 | So, at moment, partner archive is still not relevant for builds in |
| 245 | 1035 | hoary/i386. It's not passed to the builder. | 1035 | hoary/i386. It's not passed to the builder. |
| 246 | 1036 | 1036 | ||
| 248 | 1037 | >>> a_builder.dispatchBuildCandidate(candidate) | 1037 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate) |
| 249 | 1038 | ensurepresent called, url=... | 1038 | ensurepresent called, url=... |
| 250 | 1039 | ensurepresent called, | 1039 | ensurepresent called, |
| 251 | 1040 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 1040 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 252 | @@ -1084,7 +1084,8 @@ | |||
| 253 | 1084 | binary in hoary/i386, the partner archive gets included in the builder | 1084 | binary in hoary/i386, the partner archive gets included in the builder |
| 254 | 1085 | sources_list. | 1085 | sources_list. |
| 255 | 1086 | 1086 | ||
| 257 | 1087 | >>> a_builder.dispatchBuildCandidate(partner_candidate) | 1087 | >>> removeSecurityProxy( |
| 258 | 1088 | ... a_builder)._dispatchBuildCandidate(partner_candidate) | ||
| 259 | 1088 | ensurepresent called, url=... | 1089 | ensurepresent called, url=... |
| 260 | 1089 | ensurepresent called, url=http://localhost:58000/.../foo_666.dsc | 1090 | ensurepresent called, url=http://localhost:58000/.../foo_666.dsc |
| 261 | 1090 | OkSlave BUILDING | 1091 | OkSlave BUILDING |
| 262 | @@ -1126,7 +1127,7 @@ | |||
| 263 | 1126 | >>> create_binary_publication_for( | 1127 | >>> create_binary_publication_for( |
| 264 | 1127 | ... cprov_archive, hoary, PackagePublishingStatus.PUBLISHED) | 1128 | ... cprov_archive, hoary, PackagePublishingStatus.PUBLISHED) |
| 265 | 1128 | 1129 | ||
| 267 | 1129 | >>> a_builder.dispatchBuildCandidate(candidate) | 1130 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate) |
| 268 | 1130 | ensurepresent called, url=... | 1131 | ensurepresent called, url=... |
| 269 | 1131 | ensurepresent called, | 1132 | ensurepresent called, |
| 270 | 1132 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 1133 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 271 | @@ -1183,7 +1184,7 @@ | |||
| 272 | 1183 | 1184 | ||
| 273 | 1184 | This is so that the mangling tools will run over the built packages. | 1185 | This is so that the mangling tools will run over the built packages. |
| 274 | 1185 | 1186 | ||
| 276 | 1186 | >>> a_builder.dispatchBuildCandidate(candidate) | 1187 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate) |
| 277 | 1187 | ensurepresent called, url=... | 1188 | ensurepresent called, url=... |
| 278 | 1188 | ensurepresent called, | 1189 | ensurepresent called, |
| 279 | 1189 | url=http://private-ppa.launchpad.dev/cprov/ppa/ubuntu/pool/main/m/mozilla-firefox/firefox_0.9.2.orig.tar.gz | 1190 | 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 | 1223 | >>> LaunchpadZopelessLayer.switchDbUser(config.builddmaster.dbuser) | 1224 | >>> LaunchpadZopelessLayer.switchDbUser(config.builddmaster.dbuser) |
| 282 | 1224 | >>> login(ANONYMOUS) | 1225 | >>> login(ANONYMOUS) |
| 283 | 1225 | 1226 | ||
| 285 | 1226 | >>> a_builder.dispatchBuildCandidate(candidate) | 1227 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate) |
| 286 | 1227 | ensurepresent called, ... | 1228 | ensurepresent called, ... |
| 287 | 1228 | ... | 1229 | ... |
| 288 | 1229 | Ogre-component: main | 1230 | Ogre-component: main |
| 289 | @@ -1302,7 +1303,7 @@ | |||
| 290 | 1302 | >>> setupBuildQueue(candidate, a_builder) | 1303 | >>> setupBuildQueue(candidate, a_builder) |
| 291 | 1303 | >>> last_stub_mail_count = len(stub.test_emails) | 1304 | >>> last_stub_mail_count = len(stub.test_emails) |
| 292 | 1304 | 1305 | ||
| 294 | 1305 | >>> a_builder.dispatchBuildCandidate(candidate) | 1306 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate) |
| 295 | 1306 | ensurepresent called, url=... | 1307 | ensurepresent called, url=... |
| 296 | 1307 | ensurepresent called, | 1308 | ensurepresent called, |
| 297 | 1308 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 1309 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 298 | @@ -1331,7 +1332,7 @@ | |||
| 299 | 1331 | >>> create_binary_publication_for( | 1332 | >>> create_binary_publication_for( |
| 300 | 1332 | ... mark_archive, hoary, PackagePublishingStatus.PUBLISHED) | 1333 | ... mark_archive, hoary, PackagePublishingStatus.PUBLISHED) |
| 301 | 1333 | 1334 | ||
| 303 | 1334 | >>> a_builder.dispatchBuildCandidate(candidate) | 1335 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(candidate) |
| 304 | 1335 | ensurepresent called, url=... | 1336 | ensurepresent called, url=... |
| 305 | 1336 | ensurepresent called, | 1337 | ensurepresent called, |
| 306 | 1337 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 1338 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 307 | @@ -1376,7 +1377,7 @@ | |||
| 308 | 1376 | 1377 | ||
| 309 | 1377 | >>> hoary_i386.distroseries.status.name | 1378 | >>> hoary_i386.distroseries.status.name |
| 310 | 1378 | 'DEVELOPMENT' | 1379 | 'DEVELOPMENT' |
| 312 | 1379 | >>> a_builder.dispatchBuildCandidate(updates_bqItem) | 1380 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(updates_bqItem) |
| 313 | 1380 | Traceback (most recent call last): | 1381 | Traceback (most recent call last): |
| 314 | 1381 | ... | 1382 | ... |
| 315 | 1382 | 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. | 1383 | 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 | 1401 | >>> removeSecurityProxy(build).pocket = ( | 1402 | >>> removeSecurityProxy(build).pocket = ( |
| 318 | 1402 | ... PackagePublishingPocket.UPDATES) | 1403 | ... PackagePublishingPocket.UPDATES) |
| 319 | 1403 | >>> last_stub_mail_count = len(stub.test_emails) | 1404 | >>> last_stub_mail_count = len(stub.test_emails) |
| 321 | 1404 | >>> a_builder.dispatchBuildCandidate(bqItem3) | 1405 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(bqItem3) |
| 322 | 1405 | ensurepresent called, url=... | 1406 | ensurepresent called, url=... |
| 323 | 1406 | ensurepresent called, | 1407 | ensurepresent called, |
| 324 | 1407 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 1408 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 325 | @@ -1423,7 +1424,7 @@ | |||
| 326 | 1423 | >>> removeSecurityProxy(build).pocket = ( | 1424 | >>> removeSecurityProxy(build).pocket = ( |
| 327 | 1424 | ... PackagePublishingPocket.PROPOSED) | 1425 | ... PackagePublishingPocket.PROPOSED) |
| 328 | 1425 | >>> last_stub_mail_count = len(stub.test_emails) | 1426 | >>> last_stub_mail_count = len(stub.test_emails) |
| 330 | 1426 | >>> a_builder.dispatchBuildCandidate(bqItem3) | 1427 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(bqItem3) |
| 331 | 1427 | ensurepresent called, url=... | 1428 | ensurepresent called, url=... |
| 332 | 1428 | ensurepresent called, | 1429 | ensurepresent called, |
| 333 | 1429 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 1430 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 334 | @@ -1446,7 +1447,7 @@ | |||
| 335 | 1446 | >>> removeSecurityProxy(build).pocket = ( | 1447 | >>> removeSecurityProxy(build).pocket = ( |
| 336 | 1447 | ... PackagePublishingPocket.BACKPORTS) | 1448 | ... PackagePublishingPocket.BACKPORTS) |
| 337 | 1448 | >>> last_stub_mail_count = len(stub.test_emails) | 1449 | >>> last_stub_mail_count = len(stub.test_emails) |
| 339 | 1449 | >>> a_builder.dispatchBuildCandidate(bqItem3) | 1450 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(bqItem3) |
| 340 | 1450 | ensurepresent called, url=... | 1451 | ensurepresent called, url=... |
| 341 | 1451 | ensurepresent called, | 1452 | ensurepresent called, |
| 342 | 1452 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz | 1453 | url=http://localhost:58000/3/firefox_0.9.2.orig.tar.gz |
| 343 | @@ -1477,13 +1478,13 @@ | |||
| 344 | 1477 | because Embargoed-Archives and Restricted-UI implementations are not | 1478 | because Embargoed-Archives and Restricted-UI implementations are not |
| 345 | 1478 | yet ready. | 1479 | yet ready. |
| 346 | 1479 | 1480 | ||
| 348 | 1480 | >>> a_builder.dispatchBuildCandidate(bqItem3) | 1481 | >>> removeSecurityProxy(a_builder)._dispatchBuildCandidate(bqItem3) |
| 349 | 1481 | Traceback (most recent call last): | 1482 | Traceback (most recent call last): |
| 350 | 1482 | ... | 1483 | ... |
| 351 | 1483 | AssertionError: Soyuz is not yet capable of building SECURITY uploads. | 1484 | AssertionError: Soyuz is not yet capable of building SECURITY uploads. |
| 352 | 1484 | 1485 | ||
| 355 | 1485 | Builds for security pocket are marked as FAILEDTOBUILD inside | 1486 | Builds for security pocket are marked as FAILEDTOBUILD inside the |
| 356 | 1486 | findBuildCandidate method, see doc/buildd-dispatching.txt | 1487 | _findBuildCandidate() method, see doc/buildd-dispatching.txt |
| 357 | 1487 | 1488 | ||
| 358 | 1488 | 1489 | ||
| 359 | 1489 | == Builder Status Handler == | 1490 | == Builder Status Handler == |
| 360 | 1490 | 1491 | ||
| 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 | 242 | :return: A librarian file alias. | 242 | :return: A librarian file alias. |
| 366 | 243 | """ | 243 | """ |
| 367 | 244 | 244 | ||
| 368 | 245 | def findBuildCandidate(): | ||
| 369 | 246 | """Return the candidate for building. | ||
| 370 | 247 | |||
| 371 | 248 | The pending BuildQueue item with the highest score for this builder | ||
| 372 | 249 | ProcessorFamily or None if no candidate is available. | ||
| 373 | 250 | |||
| 374 | 251 | For public PPA builds, subsequent builds for a given ppa and | ||
| 375 | 252 | architecture will not be returned until the current build for | ||
| 376 | 253 | the ppa and architecture is finished. | ||
| 377 | 254 | """ | ||
| 378 | 255 | |||
| 379 | 256 | def dispatchBuildCandidate(candidate): | ||
| 380 | 257 | """Dispatch the given job to this builder. | ||
| 381 | 258 | |||
| 382 | 259 | This method can only be executed in the builddmaster machine, since | ||
| 383 | 260 | it will actually issues the XMLRPC call to the buildd-slave. | ||
| 384 | 261 | """ | ||
| 385 | 262 | |||
| 386 | 263 | def handleTimeout(logger, error_message): | 245 | def handleTimeout(logger, error_message): |
| 387 | 264 | """Handle buildd slave communication timeout situations. | 246 | """Handle buildd slave communication timeout situations. |
| 388 | 265 | 247 | ||
| 389 | @@ -274,6 +256,14 @@ | |||
| 390 | 274 | :param error_message: The error message to be used for logging. | 256 | :param error_message: The error message to be used for logging. |
| 391 | 275 | """ | 257 | """ |
| 392 | 276 | 258 | ||
| 393 | 259 | def findAndStartJob(buildd_slave=None): | ||
| 394 | 260 | """Find a job to run and send it to the buildd slave. | ||
| 395 | 261 | |||
| 396 | 262 | :param buildd_slave: An optional buildd slave that this builder should | ||
| 397 | 263 | talk to. | ||
| 398 | 264 | :return: the `IBuildQueue` instance found or None if no job was found. | ||
| 399 | 265 | """ | ||
| 400 | 266 | |||
| 401 | 277 | 267 | ||
| 402 | 278 | class IBuilderSet(Interface): | 268 | class IBuilderSet(Interface): |
| 403 | 279 | """Collections of builders. | 269 | """Collections of builders. |
| 404 | 280 | 270 | ||
| 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 | 386 | return True | 386 | return True |
| 410 | 387 | 387 | ||
| 411 | 388 | # XXX cprov 20071116: It should become part of the public | 388 | # XXX cprov 20071116: It should become part of the public |
| 413 | 389 | # findBuildCandidate once we start to detect superseded builds | 389 | # _findBuildCandidate once we start to detect superseded builds |
| 414 | 390 | # at build creation time. | 390 | # at build creation time. |
| 416 | 391 | def _findBuildCandidate(self): | 391 | def _findBinaryBuildCandidate(self): |
| 417 | 392 | """Return the highest priority build candidate for this builder. | 392 | """Return the highest priority build candidate for this builder. |
| 418 | 393 | 393 | ||
| 419 | 394 | Returns a pending IBuildQueue record queued for this builder | 394 | Returns a pending IBuildQueue record queued for this builder |
| 420 | @@ -487,10 +487,21 @@ | |||
| 421 | 487 | logger = logging.getLogger('slave-scanner') | 487 | logger = logging.getLogger('slave-scanner') |
| 422 | 488 | return logger | 488 | return logger |
| 423 | 489 | 489 | ||
| 426 | 490 | def findBuildCandidate(self): | 490 | def _findBuildCandidate(self): |
| 427 | 491 | """See `IBuilder`.""" | 491 | """Find a candidate job for dispatch to an idle buildd slave. |
| 428 | 492 | |||
| 429 | 493 | The pending BuildQueue item with the highest score for this builder | ||
| 430 | 494 | ProcessorFamily or None if no candidate is available. | ||
| 431 | 495 | |||
| 432 | 496 | For public PPA builds, subsequent builds for a given ppa and | ||
| 433 | 497 | architecture will not be returned until the current build for | ||
| 434 | 498 | the ppa and architecture is finished. | ||
| 435 | 499 | |||
| 436 | 500 | :return: A binary build candidate job. | ||
| 437 | 501 | """ | ||
| 438 | 502 | |||
| 439 | 492 | logger = self._getSlaveScannerLogger() | 503 | logger = self._getSlaveScannerLogger() |
| 441 | 493 | candidate = self._findBuildCandidate() | 504 | candidate = self._findBinaryBuildCandidate() |
| 442 | 494 | 505 | ||
| 443 | 495 | # Mark build records targeted to old source versions as SUPERSEDED | 506 | # Mark build records targeted to old source versions as SUPERSEDED |
| 444 | 496 | # and build records target to SECURITY pocket as FAILEDTOBUILD. | 507 | # and build records target to SECURITY pocket as FAILEDTOBUILD. |
| 445 | @@ -507,7 +518,7 @@ | |||
| 446 | 507 | % (build.id, candidate.id)) | 518 | % (build.id, candidate.id)) |
| 447 | 508 | build.buildstate = BuildStatus.FAILEDTOBUILD | 519 | build.buildstate = BuildStatus.FAILEDTOBUILD |
| 448 | 509 | candidate.destroySelf() | 520 | candidate.destroySelf() |
| 450 | 510 | candidate = self._findBuildCandidate() | 521 | candidate = self._findBinaryBuildCandidate() |
| 451 | 511 | continue | 522 | continue |
| 452 | 512 | 523 | ||
| 453 | 513 | publication = build.current_source_publication | 524 | publication = build.current_source_publication |
| 454 | @@ -520,7 +531,7 @@ | |||
| 455 | 520 | % (build.id, candidate.id)) | 531 | % (build.id, candidate.id)) |
| 456 | 521 | build.buildstate = BuildStatus.SUPERSEDED | 532 | build.buildstate = BuildStatus.SUPERSEDED |
| 457 | 522 | candidate.destroySelf() | 533 | candidate.destroySelf() |
| 459 | 523 | candidate = self._findBuildCandidate() | 534 | candidate = self._findBinaryBuildCandidate() |
| 460 | 524 | continue | 535 | continue |
| 461 | 525 | 536 | ||
| 462 | 526 | return candidate | 537 | return candidate |
| 463 | @@ -528,8 +539,14 @@ | |||
| 464 | 528 | # No candidate was found. | 539 | # No candidate was found. |
| 465 | 529 | return None | 540 | return None |
| 466 | 530 | 541 | ||
| 469 | 531 | def dispatchBuildCandidate(self, candidate): | 542 | def _dispatchBuildCandidate(self, candidate): |
| 470 | 532 | """See `IBuilder`.""" | 543 | """Dispatch the pending job to the associated buildd slave. |
| 471 | 544 | |||
| 472 | 545 | This method can only be executed in the builddmaster machine, since | ||
| 473 | 546 | it will actually issues the XMLRPC call to the buildd-slave. | ||
| 474 | 547 | |||
| 475 | 548 | :param candidate: The job to dispatch. | ||
| 476 | 549 | """ | ||
| 477 | 533 | logger = self._getSlaveScannerLogger() | 550 | logger = self._getSlaveScannerLogger() |
| 478 | 534 | try: | 551 | try: |
| 479 | 535 | self.startBuild(candidate, logger) | 552 | self.startBuild(candidate, logger) |
| 480 | @@ -563,6 +580,21 @@ | |||
| 481 | 563 | exc_info=True) | 580 | exc_info=True) |
| 482 | 564 | self.failbuilder(error_message) | 581 | self.failbuilder(error_message) |
| 483 | 565 | 582 | ||
| 484 | 583 | def findAndStartJob(self, buildd_slave=None): | ||
| 485 | 584 | """See IBuilder.""" | ||
| 486 | 585 | logger = self._getSlaveScannerLogger() | ||
| 487 | 586 | candidate = self._findBuildCandidate() | ||
| 488 | 587 | |||
| 489 | 588 | if candidate is None: | ||
| 490 | 589 | logger.debug("No build candidates available for builder.") | ||
| 491 | 590 | return None | ||
| 492 | 591 | |||
| 493 | 592 | if buildd_slave is not None: | ||
| 494 | 593 | self.setSlaveForTesting(buildd_slave) | ||
| 495 | 594 | |||
| 496 | 595 | self._dispatchBuildCandidate(candidate) | ||
| 497 | 596 | return candidate | ||
| 498 | 597 | |||
| 499 | 566 | 598 | ||
| 500 | 567 | class BuilderSet(object): | 599 | class BuilderSet(object): |
| 501 | 568 | """See IBuilderSet""" | 600 | """See IBuilderSet""" |
| 502 | 569 | 601 | ||
| 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 | 201 | if not builder.is_available: | 201 | if not builder.is_available: |
| 508 | 202 | self.logger.warn('builder is not available. Ignored.') | 202 | self.logger.warn('builder is not available. Ignored.') |
| 509 | 203 | continue | 203 | continue |
| 511 | 204 | candidate = builder.findBuildCandidate() | 204 | |
| 512 | 205 | candidate = builder.findAndStartJob() | ||
| 513 | 205 | if candidate is None: | 206 | if candidate is None: |
| 514 | 206 | self.logger.debug( | ||
| 515 | 207 | "No candidates available for builder.") | ||
| 516 | 208 | continue | 207 | continue |
| 517 | 209 | builder.dispatchBuildCandidate(candidate) | ||
| 518 | 210 | self.txn.commit() | 208 | self.txn.commit() |
| 519 | 211 | 209 | ||
| 520 | 212 | self.logger.info("Slave Scan Process Finished.") | 210 | self.logger.info("Slave Scan Process Finished.") |
| 521 | 213 | 211 | ||
| 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 | 6 | import unittest | 6 | import unittest |
| 527 | 7 | 7 | ||
| 528 | 8 | from zope.component import getUtility | 8 | from zope.component import getUtility |
| 529 | 9 | from zope.security.proxy import removeSecurityProxy | ||
| 530 | 9 | 10 | ||
| 531 | 10 | from canonical.testing import LaunchpadZopelessLayer | 11 | from canonical.testing import LaunchpadZopelessLayer |
| 532 | 11 | from lp.buildmaster.interfaces.buildfarmjobbehavior import ( | 12 | from lp.buildmaster.interfaces.buildfarmjobbehavior import ( |
| 534 | 12 | BuildBehaviorMismatch, IBuildFarmJobBehavior) | 13 | IBuildFarmJobBehavior) |
| 535 | 13 | from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior | 14 | from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior |
| 536 | 14 | from lp.soyuz.interfaces.archive import ArchivePurpose | 15 | from lp.soyuz.interfaces.archive import ArchivePurpose |
| 537 | 15 | from lp.soyuz.interfaces.build import BuildStatus, IBuildSet | 16 | from lp.soyuz.interfaces.build import BuildStatus, IBuildSet |
| 538 | @@ -34,13 +35,13 @@ | |||
| 539 | 34 | # Create some i386 builders ready to build PPA builds. Two | 35 | # Create some i386 builders ready to build PPA builds. Two |
| 540 | 35 | # already exist in sampledata so we'll use those first. | 36 | # already exist in sampledata so we'll use those first. |
| 541 | 36 | self.builder1 = getUtility(IBuilderSet)['bob'] | 37 | self.builder1 = getUtility(IBuilderSet)['bob'] |
| 543 | 37 | self.builder2 = getUtility(IBuilderSet)['frog'] | 38 | self.frog_builder = getUtility(IBuilderSet)['frog'] |
| 544 | 38 | self.builder3 = self.factory.makeBuilder(name='builder3') | 39 | self.builder3 = self.factory.makeBuilder(name='builder3') |
| 545 | 39 | self.builder4 = self.factory.makeBuilder(name='builder4') | 40 | self.builder4 = self.factory.makeBuilder(name='builder4') |
| 546 | 40 | self.builder5 = self.factory.makeBuilder(name='builder5') | 41 | self.builder5 = self.factory.makeBuilder(name='builder5') |
| 547 | 41 | self.builders = [ | 42 | self.builders = [ |
| 548 | 42 | self.builder1, | 43 | self.builder1, |
| 550 | 43 | self.builder2, | 44 | self.frog_builder, |
| 551 | 44 | self.builder3, | 45 | self.builder3, |
| 552 | 45 | self.builder4, | 46 | self.builder4, |
| 553 | 46 | self.builder5, | 47 | self.builder5, |
| 554 | @@ -81,7 +82,8 @@ | |||
| 555 | 81 | # there's only one builder available. | 82 | # there's only one builder available. |
| 556 | 82 | 83 | ||
| 557 | 83 | # Asking frog to find a candidate should give us the joesppa build. | 84 | # Asking frog to find a candidate should give us the joesppa build. |
| 559 | 84 | next_job = self.frog_builder.findBuildCandidate() | 85 | next_job = removeSecurityProxy( |
| 560 | 86 | self.frog_builder)._findBuildCandidate() | ||
| 561 | 85 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 87 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 562 | 86 | self.assertEqual('joesppa', build.archive.name) | 88 | self.assertEqual('joesppa', build.archive.name) |
| 563 | 87 | 89 | ||
| 564 | @@ -89,7 +91,8 @@ | |||
| 565 | 89 | # returned. | 91 | # returned. |
| 566 | 90 | self.bob_builder.builderok = False | 92 | self.bob_builder.builderok = False |
| 567 | 91 | self.bob_builder.manual = False | 93 | self.bob_builder.manual = False |
| 569 | 92 | next_job = self.frog_builder.findBuildCandidate() | 94 | next_job = removeSecurityProxy( |
| 570 | 95 | self.frog_builder)._findBuildCandidate() | ||
| 571 | 93 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 96 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 572 | 94 | self.assertEqual('joesppa', build.archive.name) | 97 | self.assertEqual('joesppa', build.archive.name) |
| 573 | 95 | 98 | ||
| 574 | @@ -162,7 +165,7 @@ | |||
| 575 | 162 | def test_findBuildCandidate_first_build_started(self): | 165 | def test_findBuildCandidate_first_build_started(self): |
| 576 | 163 | # A PPA cannot start a build if it would use 80% or more of the | 166 | # A PPA cannot start a build if it would use 80% or more of the |
| 577 | 164 | # builders. | 167 | # builders. |
| 579 | 165 | next_job = self.builder4.findBuildCandidate() | 168 | next_job = removeSecurityProxy(self.builder4)._findBuildCandidate() |
| 580 | 166 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 169 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 581 | 167 | self.failIfEqual('joesppa', build.archive.name) | 170 | self.failIfEqual('joesppa', build.archive.name) |
| 582 | 168 | 171 | ||
| 583 | @@ -170,7 +173,7 @@ | |||
| 584 | 170 | # When joe's first ppa build finishes, his fourth i386 build | 173 | # When joe's first ppa build finishes, his fourth i386 build |
| 585 | 171 | # will be the next build candidate. | 174 | # will be the next build candidate. |
| 586 | 172 | self.joe_builds[0].buildstate = BuildStatus.FAILEDTOBUILD | 175 | self.joe_builds[0].buildstate = BuildStatus.FAILEDTOBUILD |
| 588 | 173 | next_job = self.builder4.findBuildCandidate() | 176 | next_job = removeSecurityProxy(self.builder4)._findBuildCandidate() |
| 589 | 174 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 177 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 590 | 175 | self.failUnlessEqual('joesppa', build.archive.name) | 178 | self.failUnlessEqual('joesppa', build.archive.name) |
| 591 | 176 | 179 | ||
| 592 | @@ -179,7 +182,7 @@ | |||
| 593 | 179 | # for the one architecture. | 182 | # for the one architecture. |
| 594 | 180 | self.ppa_joe.private = True | 183 | self.ppa_joe.private = True |
| 595 | 181 | self.ppa_joe.buildd_secret = 'sekrit' | 184 | self.ppa_joe.buildd_secret = 'sekrit' |
| 597 | 182 | next_job = self.builder4.findBuildCandidate() | 185 | next_job = removeSecurityProxy(self.builder4)._findBuildCandidate() |
| 598 | 183 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 186 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 599 | 184 | self.failUnlessEqual('joesppa', build.archive.name) | 187 | self.failUnlessEqual('joesppa', build.archive.name) |
| 600 | 185 | 188 | ||
| 601 | @@ -205,7 +208,8 @@ | |||
| 602 | 205 | # Normal archives are not restricted to serial builds per | 208 | # Normal archives are not restricted to serial builds per |
| 603 | 206 | # arch. | 209 | # arch. |
| 604 | 207 | 210 | ||
| 606 | 208 | next_job = self.builder2.findBuildCandidate() | 211 | next_job = removeSecurityProxy( |
| 607 | 212 | self.frog_builder)._findBuildCandidate() | ||
| 608 | 209 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 213 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 609 | 210 | self.failUnlessEqual('primary', build.archive.name) | 214 | self.failUnlessEqual('primary', build.archive.name) |
| 610 | 211 | self.failUnlessEqual('gedit', build.sourcepackagerelease.name) | 215 | self.failUnlessEqual('gedit', build.sourcepackagerelease.name) |
| 611 | @@ -213,8 +217,9 @@ | |||
| 612 | 213 | # Now even if we set the build building, we'll still get the | 217 | # Now even if we set the build building, we'll still get the |
| 613 | 214 | # second non-ppa build for the same archive as the next candidate. | 218 | # second non-ppa build for the same archive as the next candidate. |
| 614 | 215 | build.buildstate = BuildStatus.BUILDING | 219 | build.buildstate = BuildStatus.BUILDING |
| 617 | 216 | build.builder = self.builder2 | 220 | build.builder = self.frog_builder |
| 618 | 217 | next_job = self.builder2.findBuildCandidate() | 221 | next_job = removeSecurityProxy( |
| 619 | 222 | self.frog_builder)._findBuildCandidate() | ||
| 620 | 218 | build = getUtility(IBuildSet).getByQueueEntry(next_job) | 223 | build = getUtility(IBuildSet).getByQueueEntry(next_job) |
| 621 | 219 | self.failUnlessEqual('primary', build.archive.name) | 224 | self.failUnlessEqual('primary', build.archive.name) |
| 622 | 220 | self.failUnlessEqual('firefox', build.sourcepackagerelease.name) | 225 | self.failUnlessEqual('firefox', build.sourcepackagerelease.name) |
Hello there!
This branch cleans up the IBuilder interface and model code related to
candidate job selection.
The findBuildCandid
and a new findAndStartJob() method that hides them is introduced.
Tests to run:
bin/test -vv -t build
No "make lint" errors.