Merge lp:~james-w/launchpad/create-source-recipe-build into lp:launchpad

Proposed by James Westby
Status: Merged
Merged at revision: not available
Proposed branch: lp:~james-w/launchpad/create-source-recipe-build
Merge into: lp:launchpad
Prerequisite: lp:~wgrant/launchpad/sprb-createbuildqueueentry-creation
Diff against target: 268 lines (+132/-13)
6 files modified
lib/lp/archiveuploader/permission.py (+19/-10)
lib/lp/archiveuploader/tests/test_permission.py (+8/-0)
lib/lp/archiveuploader/uploadprocessor.py (+0/-3)
lib/lp/soyuz/interfaces/sourcepackagerecipe.py (+10/-0)
lib/lp/soyuz/model/sourcepackagerecipe.py (+26/-0)
lib/lp/soyuz/tests/test_sourcepackagerecipe.py (+69/-0)
To merge this branch: bzr merge lp:~james-w/launchpad/create-source-recipe-build
Reviewer Review Type Date Requested Status
Julian Edwards (community) code Approve
Review via email: mp+17435@code.launchpad.net
To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

This adds a requestBuild method to SourcePackageRecipe, allowing you
to request that the recipe be built in to a specific archive.

There is a TODO which can be removed and the assertion uncommented
when the prerequisite branch is fixed to implement the interface
correctly.

There are a few other changes necessary to do this elegantly:

  The objects returned from check_upload_to_archive now derive from
  Exception so that we can raise them directly. This shouldn't affect
  existing code.

  check_upload_to_archive now checks for disabled archives, and this
  is moved out of processupload.py

Thanks,

James

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Looks good, just a few small tweaks. Landing is conditional on fixing these and your TODO!

* Please document the fact that requestBuild raises exceptions in the interface docstring.
* it's "targeted" not "targetted"
* Our coding standards say that you should format method invocations like this:

    method(
        arg, arg, arg)

 i.e. newline after the opening parens.

* Also indent 4 chars only

Nice tests!

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/archiveuploader/permission.py'
--- lib/lp/archiveuploader/permission.py 2009-11-20 22:22:53 +0000
+++ lib/lp/archiveuploader/permission.py 2010-01-15 01:34:29 +0000
@@ -20,24 +20,21 @@
20from lp.soyuz.interfaces.archive import ArchivePurpose20from lp.soyuz.interfaces.archive import ArchivePurpose
2121
2222
23class CannotUploadToArchive:23class CannotUploadToArchive(Exception):
24 """A reason for not being able to upload to an archive."""24 """A reason for not being able to upload to an archive."""
2525
26 _fmt = '%(person)s has no upload rights to %(archive)s.'26 _fmt = '%(person)s has no upload rights to %(archive)s.'
2727
28 def __init__(self, **args):28 def __init__(self, **args):
29 """Construct a `CannotUploadToArchive`."""29 """Construct a `CannotUploadToArchive`."""
30 self._message = self._fmt % args30 Exception.__init__(self, self._fmt % args)
3131
32 def __str__(self):32
33 return self._message33class CannotUploadToPocket(Exception):
34
35
36class CannotUploadToPocket:
37 """Returned when a pocket is closed for uploads."""34 """Returned when a pocket is closed for uploads."""
3835
39 def __init__(self, distroseries, pocket):36 def __init__(self, distroseries, pocket):
40 super(CannotUploadToPocket, self).__init__(37 Exception.__init__(self,
41 "Not permitted to upload to the %s pocket in a series in the "38 "Not permitted to upload to the %s pocket in a series in the "
42 "'%s' state." % (pocket.name, distroseries.status.name))39 "'%s' state." % (pocket.name, distroseries.status.name))
4340
@@ -72,7 +69,7 @@
72 "Signer is not permitted to upload to the component '%(component)s'.")69 "Signer is not permitted to upload to the component '%(component)s'.")
7370
74 def __init__(self, component):71 def __init__(self, component):
75 super(NoRightsForComponent, self).__init__(component=component.name)72 CannotUploadToArchive.__init__(self, component=component.name)
7673
7774
78class InvalidPocketForPPA(CannotUploadToArchive):75class InvalidPocketForPPA(CannotUploadToArchive):
@@ -87,6 +84,15 @@
87 _fmt = "Partner uploads must be for the RELEASE or PROPOSED pocket."84 _fmt = "Partner uploads must be for the RELEASE or PROPOSED pocket."
8885
8986
87class ArchiveDisabled(CannotUploadToArchive):
88 """Uploading to a disabled archive is not allowed."""
89
90 _fmt = ("%(archive_name)s is disabled.")
91
92 def __init__(self, archive_name):
93 CannotUploadToArchive.__init__(self, archive_name=archive_name)
94
95
90def components_valid_for(archive, person):96def components_valid_for(archive, person):
91 """Return the components that 'person' can upload to 'archive'.97 """Return the components that 'person' can upload to 'archive'.
9298
@@ -190,6 +196,9 @@
190 :return: CannotUploadToArchive if 'person' cannot upload to the archive,196 :return: CannotUploadToArchive if 'person' cannot upload to the archive,
191 None otherwise.197 None otherwise.
192 """198 """
199 if not archive.enabled:
200 return ArchiveDisabled(archive.displayname)
201
193 # For PPAs...202 # For PPAs...
194 if archive.is_ppa:203 if archive.is_ppa:
195 if not archive.canUpload(person):204 if not archive.canUpload(person):
196205
=== modified file 'lib/lp/archiveuploader/tests/test_permission.py'
--- lib/lp/archiveuploader/tests/test_permission.py 2009-11-20 22:22:53 +0000
+++ lib/lp/archiveuploader/tests/test_permission.py 2010-01-15 01:34:29 +0000
@@ -241,6 +241,14 @@
241 self.assertCanUpload(241 self.assertCanUpload(
242 person, spn, archive, component_a, strict_component=False)242 person, spn, archive, component_a, strict_component=False)
243243
244 def test_cannot_upload_to_disabled_archive(self):
245 spn = self.factory.makeSourcePackageName()
246 archive = self.factory.makeArchive()
247 removeSecurityProxy(archive).disable()
248 component = self.factory.makeComponent()
249 self.assertCannotUpload(u"%s is disabled." % (archive.displayname),
250 archive.owner, spn, archive, component)
251
244252
245def test_suite():253def test_suite():
246 return unittest.TestLoader().loadTestsFromName(__name__)254 return unittest.TestLoader().loadTestsFromName(__name__)
247255
=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py 2009-06-24 23:33:29 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py 2010-01-15 01:34:29 +0000
@@ -587,9 +587,6 @@
587 # Upload path does not match anything we support.587 # Upload path does not match anything we support.
588 raise UploadPathError("Path format mismatch.")588 raise UploadPathError("Path format mismatch.")
589589
590 if not archive.enabled:
591 raise PPAUploadPathError("%s is disabled." % archive.displayname)
592
593 if archive.distribution != distribution:590 if archive.distribution != distribution:
594 raise PPAUploadPathError(591 raise PPAUploadPathError(
595 "%s only supports uploads to '%s' distribution."592 "%s only supports uploads to '%s' distribution."
596593
=== modified file 'lib/lp/soyuz/interfaces/sourcepackagerecipe.py'
--- lib/lp/soyuz/interfaces/sourcepackagerecipe.py 2010-01-15 01:34:24 +0000
+++ lib/lp/soyuz/interfaces/sourcepackagerecipe.py 2010-01-15 01:34:29 +0000
@@ -77,6 +77,16 @@
77 def getReferencedBranches():77 def getReferencedBranches():
78 """An iterator of the branches referenced by this recipe."""78 """An iterator of the branches referenced by this recipe."""
7979
80 def requestBuild(self, archive, distroseries, requester, pocket):
81 """Request that the recipe be built in to the specified archive.
82
83 :param archive: The IArchive which you want the build to end up in.
84 :param distroseries: The DistroSeries that the package should be
85 targetted to.
86 :param requester: the person requesting the build.
87 :param pocket: the pocket that should be targetted.
88 """
89
8090
81class ISourcePackageRecipeSource(Interface):91class ISourcePackageRecipeSource(Interface):
82 """A utility of this interface can be used to create and access recipes.92 """A utility of this interface can be used to create and access recipes.
8393
=== modified file 'lib/lp/soyuz/model/sourcepackagerecipe.py'
--- lib/lp/soyuz/model/sourcepackagerecipe.py 2010-01-15 01:34:24 +0000
+++ lib/lp/soyuz/model/sourcepackagerecipe.py 2010-01-15 01:34:29 +0000
@@ -10,16 +10,26 @@
1010
11from storm.locals import Int, Reference, Store, Storm, Unicode11from storm.locals import Int, Reference, Store, Storm, Unicode
1212
13from zope.component import getUtility
13from zope.interface import classProvides, implements14from zope.interface import classProvides, implements
1415
15from canonical.database.datetimecol import UtcDateTimeCol16from canonical.database.datetimecol import UtcDateTimeCol
16from canonical.launchpad.interfaces.lpstorm import IMasterStore17from canonical.launchpad.interfaces.lpstorm import IMasterStore
1718
19from lp.soyuz.interfaces.archive import ArchivePurpose
20from lp.soyuz.interfaces.component import IComponentSet
21from lp.archiveuploader.permission import check_upload_to_archive
18from lp.soyuz.interfaces.sourcepackagerecipe import (22from lp.soyuz.interfaces.sourcepackagerecipe import (
19 ISourcePackageRecipe, ISourcePackageRecipeSource)23 ISourcePackageRecipe, ISourcePackageRecipeSource)
24from lp.soyuz.interfaces.sourcepackagerecipebuild import (
25 ISourcePackageRecipeBuildSource)
20from lp.soyuz.model.sourcepackagerecipedata import _SourcePackageRecipeData26from lp.soyuz.model.sourcepackagerecipedata import _SourcePackageRecipeData
2127
2228
29class NonPPABuildRequest(Exception):
30 """A build was requested to a non-PPA and this is currently unsupported."""
31
32
23class SourcePackageRecipe(Storm):33class SourcePackageRecipe(Storm):
24 """See `ISourcePackageRecipe` and `ISourcePackageRecipeSource`."""34 """See `ISourcePackageRecipe` and `ISourcePackageRecipeSource`."""
2535
@@ -82,3 +92,19 @@
82 sprecipe.name = name92 sprecipe.name = name
83 store.add(sprecipe)93 store.add(sprecipe)
84 return sprecipe94 return sprecipe
95
96 def requestBuild(self, archive, requester, pocket):
97 if archive.purpose != ArchivePurpose.PPA:
98 raise NonPPABuildRequest
99 component = getUtility(IComponentSet)["multiverse"]
100 reject_reason = check_upload_to_archive(requester,
101 self.distroseries, self.sourcepackagename,
102 archive, component, pocket)
103 if reject_reason is not None:
104 raise reject_reason
105 sourcepackage = self.distroseries.getSourcePackage(
106 self.sourcepackagename)
107 build = getUtility(ISourcePackageRecipeBuildSource).new(sourcepackage,
108 self, requester, archive)
109 build.createBuildQueueEntry()
110 return build
85111
=== modified file 'lib/lp/soyuz/tests/test_sourcepackagerecipe.py'
--- lib/lp/soyuz/tests/test_sourcepackagerecipe.py 2010-01-15 01:34:24 +0000
+++ lib/lp/soyuz/tests/test_sourcepackagerecipe.py 2010-01-15 01:34:29 +0000
@@ -10,14 +10,33 @@
1010
11from bzrlib.plugins.builder.recipe import RecipeParser11from bzrlib.plugins.builder.recipe import RecipeParser
1212
13from storm.locals import Store
14
13from zope.component import getUtility15from zope.component import getUtility
14from zope.security.interfaces import Unauthorized16from zope.security.interfaces import Unauthorized
17from zope.security.proxy import removeSecurityProxy
1518
16from canonical.testing.layers import DatabaseFunctionalLayer19from canonical.testing.layers import DatabaseFunctionalLayer
1720
21from lp.archiveuploader.permission import (
22 ArchiveDisabled, CannotUploadToArchive, InvalidPocketForPPA)
23from lp.registry.interfaces.pocket import PackagePublishingPocket
24from lp.services.job.interfaces.job import (
25 IJob, JobStatus)
26from lp.soyuz.interfaces.archive import ArchivePurpose
27from lp.soyuz.interfaces.buildqueue import (
28 IBuildQueue)
18from lp.soyuz.interfaces.sourcepackagerecipe import (29from lp.soyuz.interfaces.sourcepackagerecipe import (
19 ForbiddenInstruction, ISourcePackageRecipe, ISourcePackageRecipeSource,30 ForbiddenInstruction, ISourcePackageRecipe, ISourcePackageRecipeSource,
20 TooNewRecipeFormat)31 TooNewRecipeFormat)
32from lp.soyuz.interfaces.sourcepackagerecipebuild import (
33 ISourcePackageRecipeBuild, ISourcePackageRecipeBuildJob)
34from lp.soyuz.model.buildqueue import (
35 BuildQueue)
36from lp.soyuz.model.sourcepackagerecipebuild import (
37 SourcePackageRecipeBuildJob)
38from lp.soyuz.model.sourcepackagerecipe import (
39 NonPPABuildRequest)
21from lp.testing import login_person, TestCaseWithFactory40from lp.testing import login_person, TestCaseWithFactory
2241
2342
@@ -154,6 +173,56 @@
154 TooNewRecipeFormat,173 TooNewRecipeFormat,
155 self.makeSourcePackageRecipeFromBuilderRecipe, builder_recipe)174 self.makeSourcePackageRecipeFromBuilderRecipe, builder_recipe)
156175
176 def test_requestBuild(self):
177 recipe = self.factory.makeSourcePackageRecipe()
178 ppa = self.factory.makeArchive()
179 build = recipe.requestBuild(ppa, ppa.owner,
180 PackagePublishingPocket.RELEASE)
181 # TODO: Fails as SourcePackageRecipeBuild doesn't correctly
182 # implement the interface currently.
183 #self.assertProvides(build, ISourcePackageRecipeBuild)
184 self.assertEqual(build.archive, ppa)
185 self.assertEqual(build.distroseries, recipe.distroseries)
186 self.assertEqual(build.requester, ppa.owner)
187 store = Store.of(build)
188 store.flush()
189 build_job = store.find(SourcePackageRecipeBuildJob,
190 SourcePackageRecipeBuildJob.build_id==build.id).one()
191 self.assertProvides(build_job, ISourcePackageRecipeBuildJob)
192 self.assertTrue(build_job.virtualized)
193 job = build_job.job
194 self.assertProvides(job, IJob)
195 self.assertEquals(job.status, JobStatus.WAITING)
196 build_queue = store.find(BuildQueue, BuildQueue.job==job.id).one()
197 self.assertProvides(build_queue, IBuildQueue)
198 self.assertTrue(build_queue.virtualized)
199
200 def test_requestBuildRejectsNotPPA(self):
201 recipe = self.factory.makeSourcePackageRecipe()
202 not_ppa = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
203 self.assertRaises(NonPPABuildRequest, recipe.requestBuild, not_ppa,
204 not_ppa.owner, PackagePublishingPocket.RELEASE)
205
206 def test_requestBuildRejectsNoPermission(self):
207 recipe = self.factory.makeSourcePackageRecipe()
208 ppa = self.factory.makeArchive()
209 requester = self.factory.makePerson()
210 self.assertRaises(CannotUploadToArchive, recipe.requestBuild, ppa,
211 requester, PackagePublishingPocket.RELEASE)
212
213 def test_requestBuildRejectsInvalidPocket(self):
214 recipe = self.factory.makeSourcePackageRecipe()
215 ppa = self.factory.makeArchive()
216 self.assertRaises(InvalidPocketForPPA, recipe.requestBuild, ppa,
217 ppa.owner, PackagePublishingPocket.BACKPORTS)
218
219 def test_requestBuildRejectsDisabledArchive(self):
220 recipe = self.factory.makeSourcePackageRecipe()
221 ppa = self.factory.makeArchive()
222 removeSecurityProxy(ppa).disable()
223 self.assertRaises(ArchiveDisabled, recipe.requestBuild, ppa,
224 ppa.owner, PackagePublishingPocket.RELEASE)
225
157226
158class TestRecipeBranchRoundTripping(TestCaseWithFactory):227class TestRecipeBranchRoundTripping(TestCaseWithFactory):
159228