Launchpad itself

lib/lp/buildmaster/model/buildbase.py (+5/-4)
lib/lp/buildmaster/model/builder.py (+3/-3)
lib/lp/soyuz/doc/buildd-slavescanner.txt (+1/-7)
lib/lp/translations/model/translationtemplatesbuildbehavior.py (+11/-7)
lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py (+1/-1)

To merge this branch:

bzr merge lp://staging/~wgrant/launchpad/bug-549907-stop-using-slave-build-id

Low

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Nelson (community)	code	2010-04-03	Approve on 2010-04-08
Jelmer Vernooij (community)	code*	2010-04-01	Approve on 2010-04-03
Review via email: mp+22585@code.staging.launchpad.net

Commit message

Restrict the master's use of slave build IDs to rescueBuilderIfLost.

Description of the change

The build farm slave build ID (returned in the slave status string) can be falsified by the easily hijacked slaves. It should therefore not be used for anything besides its primary purpose -- rescueIfLost.

However, it's currently used in a few log messages and a directory name. This branch fixes them to use something else, and removes the slave build ID from the slave status dict, making it rather difficult for new code to start using it without trying REALLY hard.

The upload directory change is not significant, since it simply generates the same correct ID itself, rather than trusting the one from the slave.

Revision history for this message

Jelmer Vernooij (jelmer) wrote on 2010-04-03:

Thanks. Other than reducing the amount of bookkeeping in various places, I think the build title is also generally a more useful string.

I see you removed the references to success / successful in lib/lp/translations/model/translationtemplatesbuildbehavior.py; it seems useful to mention this, though perhaps it's sufficient that the previous log line would also mention the fact that the status of the build was ok.

review: Approve (code*)

Revision history for this message

William Grant (wgrant) wrote on 2010-04-03:

> Thanks. Other than reducing the amount of bookkeeping in various places, I
> think the build title is also generally a more useful string.

Thanks for the review.

> I see you removed the references to success / successful in
> lib/lp/translations/model/translationtemplatesbuildbehavior.py; it seems
> useful to mention this, though perhaps it's sufficient that the previous log
> line would also mention the fact that the status of the build was ok.

Right, since it's all synchronous the logging will go something like this:

Templates generation job %s for %s finished with status %s.
Processing successful templates build.
Build produced no tarball.

Revision history for this message

Michael Nelson (michael.nelson) wrote on 2010-04-08:

Unstarred :)

review: Approve (code)