Merge lp:~michael.nelson/launchpad/567922-binarypackagebuild-new-table-2 into lp:launchpad/db-devel
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Michael Nelson on 2010-05-06 | ||||
| Approved revision: | no longer in the source branch. | ||||
| Merged at revision: | 9405 | ||||
| Proposed branch: | lp:~michael.nelson/launchpad/567922-binarypackagebuild-new-table-2 | ||||
| Merge into: | lp:launchpad/db-devel | ||||
| Prerequisite: | lp:~michael.nelson/launchpad/567922-binarypackagebuild-new-table-1 | ||||
| Diff against target: |
756 lines (+188/-86) (has conflicts) 17 files modified
lib/lp/buildmaster/interfaces/buildbase.py (+7/-1) lib/lp/buildmaster/interfaces/packagebuild.py (+7/-0) lib/lp/buildmaster/model/buildbase.py (+6/-5) lib/lp/buildmaster/model/buildfarmjob.py (+1/-1) lib/lp/buildmaster/model/packagebuild.py (+11/-4) lib/lp/code/model/sourcepackagerecipe.py (+1/-1) lib/lp/code/tests/test_sourcepackagerecipebuild.py (+1/-1) lib/lp/soyuz/interfaces/binarypackagebuild.py (+2/-1) lib/lp/soyuz/model/binarypackagebuild.py (+48/-29) lib/lp/soyuz/model/buildfarmbuildjob.py (+50/-0) lib/lp/soyuz/model/buildpackagejob.py (+7/-7) lib/lp/soyuz/model/publishing.py (+1/-1) lib/lp/soyuz/model/sourcepackagerelease.py (+15/-8) lib/lp/soyuz/tests/test_binarypackagebuild.py (+11/-9) lib/lp/soyuz/tests/test_hasbuildrecords.py (+3/-1) lib/lp/soyuz/tests/test_publishing.py (+14/-14) lib/lp/soyuz/tests/test_publishing_models.py (+3/-3) 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:~michael.nelson/launchpad/567922-binarypackagebuild-new-table-2 | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Brad Crittenden (community) | code | 2010-05-04 | Approve on 2010-05-04 |
|
Review via email:
|
|||
Description of the Change
This branch is part of a pipeline for
https:/
https:/
Overview
========
This branch continues the work to switch our BinaryPackageBuild class to the new binarypackagebuild table (using the delegated PackageBuild/
Details
=======
Renames test_build to test_binarypack
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_
bin/py database/
And then:
bin/test -vv -m test_binarypack
The next branch will continue to update the BinaryPackageBuild implementation to get the rest of the tests working.
| Michael Nelson (michael.nelson) wrote : | # |
| Brad Crittenden (bac) wrote : | # |
Hi Michael,
Man I know you're going to be happy to get these changes all wrapped
up. Thanks for doing a very tedious series of branches.
I'm glad to see soyuzvariablenames expanded. thank_you.
--bac
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -151,6 +151,13 @@
> :param slave_status: A dict as returned by IBuilder.
> """
>
> + def queueBuild(
> + """Create a BuildQueue entry for this build.
> +
> + :param suspended: Whether the associated `Job` instance should be
> + created in a suspended state.
> + """
So interface method specifications leave off the 'self' argument but
as a staticmethod there is no self, so the first argument 'build' is
dropped? That's no good.
>
> class IPackageBuildSo
> """A utility of this interface used to create _things_."""
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -189,7 +189,7 @@
> def __init__(self, job_type, status=
> processor=None, virtualized=None):
> super(BuildFarmJob, self).__init__()
> - self.job_type, self.status, self.process, self.virtualized = (
> + self.job_type, self.status, self.processor, self.virtualized = (
> job_type,
> status,
> processor,
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -778,7 +791,7 @@
>
> # This code MUST match the logic in the Build security adapter,
> # otherwise users are likely to get 403 errors, or worse.
If it MUST match then perhaps it should be refactored and shared?
(Not a requirement for this branch.)
> === renamed file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -7,6 +7,7 @@
>
> from storm.store import Store
> from zope.component import getUtility
> +from zope.security.proxy import removeSecurityProxy
>
> from canonical.
> from canonical.testing import LaunchpadZopele
> @@ -81,7 +82,7 @@
>
> [depwait_build] = depwait_
> depwait_
> - depwait_
> + depwait_
Why did you need to convert these strings to unicode?
| Michael Nelson (michael.nelson) wrote : | # |
On Tue, May 4, 2010 at 7:52 PM, Brad Crittenden <email address hidden> wrote:
> Review: Approve code
> Hi Michael,
>
> Man I know you're going to be happy to get these changes all wrapped
> up. Thanks for doing a very tedious series of branches.
>
> I'm glad to see soyuzvariablenames expanded. thank_you.
>
> --bac
Thanks for the review bac, a few comments below.
>
>
>> === modified file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -151,6 +151,13 @@
>> :param slave_status: A dict as returned by IBuilder.
>> """
>>
>> + def queueBuild(
>> + """Create a BuildQueue entry for this build.
>> +
>> + :param suspended: Whether the associated `Job` instance should be
>> + created in a suspended state.
>> + """
>
> So interface method specifications leave off the 'self' argument but
> as a staticmethod there is no self, so the first argument 'build' is
> dropped? That's no good.
It's not a static method on PackageBuild... the
PackageBuild.
a static method (so the implementation can be shared in the transition
by old BuildBase-based classes and the new PackageBuild classes).
Let me know if that was obvious to you and there is still something
you'd like to see changed?
>
>>
>> class IPackageBuildSo
>> """A utility of this interface used to create _things_."""
>
>> === modified file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -189,7 +189,7 @@
>> def __init__(self, job_type, status=
>> processor=None, virtualized=None):
>> super(
>> - self.job_type, self.status, self.process, self.virtualized = (
>> + self.job_type, self.status, self.processor, self.virtualized = (
>> job_type,
>> status,
>> processor,
>
>> === modified file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>
>> @@ -778,7 +791,7 @@
>>
>> # This code MUST match the logic in the Build security adapter,
>> # otherwise users are likely to get 403 errors, or worse.
>
> If it MUST match then perhaps it should be refactored and shared?
> (Not a requirement for this branch.)
Yeah, I thought the same thing when I moved that comment.
>
>> === renamed file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>> @@ -7,6 +7,7 @@
>>
>> from storm.store import Store
>> from zope.component import getUtility
>> +from zope.security.proxy import r...

Diff here: http:// pastebin. ubuntu. com/427743/ (in case the scanner is still catching up).