Launchpad itself

lib/lp/buildmaster/interfaces/buildbase.py (+10/-1)
lib/lp/buildmaster/interfaces/buildfarmjob.py (+11/-1)
lib/lp/buildmaster/interfaces/packagebuild.py (+43/-0)
lib/lp/buildmaster/model/buildbase.py (+58/-28)
lib/lp/buildmaster/model/buildfarmjob.py (+4/-0)
lib/lp/buildmaster/model/packagebuild.py (+33/-0)
lib/lp/buildmaster/tests/test_buildbase.py (+61/-12)
lib/lp/buildmaster/tests/test_buildfarmjob.py (+1/-0)
lib/lp/buildmaster/tests/test_packagebuild.py (+38/-12)
lib/lp/soyuz/doc/buildd-slavescanner.txt (+2/-1)
lib/lp/soyuz/tests/test_build.py (+32/-13)

Text conflict in lib/lp/buildmaster/interfaces/buildbase.py
Text conflict in lib/lp/buildmaster/model/buildbase.py
Text conflict in lib/lp/buildmaster/tests/test_buildbase.py

To merge this branch:

bzr merge lp://staging/~michael.nelson/launchpad/567922-binarypackagebuild-packagebuild-2

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Jeroen T. Vermeulen (community)	code	2010-04-27	Approve on 2010-04-29
Brad Crittenden (community)		2010-04-27	Abstain on 2010-04-27
Jelmer Vernooij	code	2010-04-27	Pending
Review via email: mp+24211@code.staging.launchpad.net

Description of the change

This branch is part of a pipeline for

https://blueprints.edge.launchpad.net/soyuz/+spec/build-generalisation
https://dev.launchpad.net/LEP/GeneralBuildHistories

Overview
========
The changes in this branch continue the preparation to switch the BinaryPackageBuild class from the old build table to the new collection of 3 tables (buildfarmjob, packagebuild and binarypackagebuild, similar to http://people.ubuntu.com/~wgrant/launchpad/buildfarm/new-build-model-again.png).

The previous MP started this (adding the attributes with specific API versions):
https://code.edge.launchpad.net/~michael.nelson/launchpad/567922-binarypackagebuild-packagebuild/+merge/24107

This branch mainly consists of making sure that the BuildFarmJob/PackageBuild classes provide the same interface methods as the current IBuildBase (which is *not* a database class), while at the same time,

(1) ensuring IBuildBase (and its implementation) still function correctly as it is used by other classes which will be updated in later work (ie. SourcePackageRecipeBuild), and
(2) we don't duplicate the implementation during the transition (so that there's no possibility of divergence).

To achieve these two points, I had to turn some of the BuildBase methods into staticmethods with the build as the first argument so the implementation can be used by both IBuildBase and IPackageBuild.

This branch is dependent on the pending schema patch in a previous branch.

To test
=======

First update the test db schema (required as the db patch still needs to be updated to remove the old build table):
psql launchpad_ftest_template -f database/schema/pending/michaeln-build-generalisation.sql
bin/py database/schema/security.py -d launchpad_ftest_template

And then:
bin/test -vvt test_packagebuild -t test_buildfarmjob -t doc/build.txt -t test_buildqueue -t test_sourcepackagerecipebuild -t test_buildpackagejob -t test_buildbase

(there is one intentional test failure in test_buildfarmjob for a method that can't be implemented until I switch BinaryPackageBuild to the new delegation model of IPackageBuild/IBuildFarmJob).

The next branch will continue to transfer the interface to IBuildFarmJob/IPackageBuild before finally switching the BinaryPackageBuild from the old build table to the new binarypackagebuild table (Note: IBuildBase will still be used by ISourcePackageRecipeBuild until we do a second db-schema change).

Outstanding questions
=================
Because BuildBase is currently shared by SPRecipeBuild and BinaryPackageBuild, I've basically just ensure that the equivalently shared model in the new schema - IPackageBuild - gets these attributes/methods, but they could sensibly be on IBuildFarmJob instead, so that they were available for all build farm jobs (ie. including TranslationsTemplateBuildJob)... I hope to get some input from jtv on that.

Revision history for this message

Brad Crittenden (bac) wrote on 2010-04-27:

Michael I had some questions I wanted to discuss with you on IRC to help me understand what you're doing here.

Since you aren't around due to hitting EOD I am going to abstain on this branch so you can get a review first thing tomorrow, hopefully by someone who is already familiar with this effort.

review: Abstain

Revision history for this message

Jeroen T. Vermeulen (jtv) wrote on 2010-04-29:

Discussed on IRC. A fair portion of my notes were on docstrings that aren't new and have been handed down from times immemorial; might as well seize the opportunity to clear things up while we're here.

lib/lp/buildmaster/interfaces/buildfarmjob.py: makeJob

I thought this would be a problem with TranslationTemplatesBuildJob, which is based on BranchJob whose constructor likes to create its own Jobs. Turns out this has been taken into account, but the docstring has been clarified. Thanks!

lib/lp/buildmaster/interfaces/packagebuild.py: getUploaderCommand

Clarified: what does "the command to run as the uploader" mean? That all this is about on command, and it is to be run under the uploader's identity?

lib/lp/buildmaster/interfaces/packagebuild.py: getUploadLeaf

Docstring clarified.

The slave build id has been replaced with the slave build cookie. It is now a hash without (I hope) any distinguishable components.

lib/lp/buildmaster/tests/test_buildbase.py: BuildBaseTestCase

Don't derive base class for tests from TestCase. It's not a test. Make it a mixin instead, and use multiple inheritance in the leaf classes. Also saves you from deriving a test case from both TestCase *and* TestCaseWithFactory. Rename setUp so as not to confuse super().

At the time of writing, this is being resolved.

Good luck with this refactoring!

Jeroen

review: Approve (code)