Launchpad itself

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

  1. create-source-recipe-build
  2. Merge into devel
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
1=== modified file 'lib/lp/archiveuploader/permission.py'
2--- lib/lp/archiveuploader/permission.py 2009-11-20 22:22:53 +0000
3+++ lib/lp/archiveuploader/permission.py 2010-01-15 01:34:29 +0000
4@@ -20,24 +20,21 @@
5 from lp.soyuz.interfaces.archive import ArchivePurpose
6
7
8-class CannotUploadToArchive:
9+class CannotUploadToArchive(Exception):
10 """A reason for not being able to upload to an archive."""
11
12 _fmt = '%(person)s has no upload rights to %(archive)s.'
13
14 def __init__(self, **args):
15 """Construct a `CannotUploadToArchive`."""
16- self._message = self._fmt % args
17-
18- def __str__(self):
19- return self._message
20-
21-
22-class CannotUploadToPocket:
23+ Exception.__init__(self, self._fmt % args)
24+
25+
26+class CannotUploadToPocket(Exception):
27 """Returned when a pocket is closed for uploads."""
28
29 def __init__(self, distroseries, pocket):
30- super(CannotUploadToPocket, self).__init__(
31+ Exception.__init__(self,
32 "Not permitted to upload to the %s pocket in a series in the "
33 "'%s' state." % (pocket.name, distroseries.status.name))
34
35@@ -72,7 +69,7 @@
36 "Signer is not permitted to upload to the component '%(component)s'.")
37
38 def __init__(self, component):
39- super(NoRightsForComponent, self).__init__(component=component.name)
40+ CannotUploadToArchive.__init__(self, component=component.name)
41
42
43 class InvalidPocketForPPA(CannotUploadToArchive):
44@@ -87,6 +84,15 @@
45 _fmt = "Partner uploads must be for the RELEASE or PROPOSED pocket."
46
47
48+class ArchiveDisabled(CannotUploadToArchive):
49+ """Uploading to a disabled archive is not allowed."""
50+
51+ _fmt = ("%(archive_name)s is disabled.")
52+
53+ def __init__(self, archive_name):
54+ CannotUploadToArchive.__init__(self, archive_name=archive_name)
55+
56+
57 def components_valid_for(archive, person):
58 """Return the components that 'person' can upload to 'archive'.
59
60@@ -190,6 +196,9 @@
61 :return: CannotUploadToArchive if 'person' cannot upload to the archive,
62 None otherwise.
63 """
64+ if not archive.enabled:
65+ return ArchiveDisabled(archive.displayname)
66+
67 # For PPAs...
68 if archive.is_ppa:
69 if not archive.canUpload(person):
70
71=== modified file 'lib/lp/archiveuploader/tests/test_permission.py'
72--- lib/lp/archiveuploader/tests/test_permission.py 2009-11-20 22:22:53 +0000
73+++ lib/lp/archiveuploader/tests/test_permission.py 2010-01-15 01:34:29 +0000
74@@ -241,6 +241,14 @@
75 self.assertCanUpload(
76 person, spn, archive, component_a, strict_component=False)
77
78+ def test_cannot_upload_to_disabled_archive(self):
79+ spn = self.factory.makeSourcePackageName()
80+ archive = self.factory.makeArchive()
81+ removeSecurityProxy(archive).disable()
82+ component = self.factory.makeComponent()
83+ self.assertCannotUpload(u"%s is disabled." % (archive.displayname),
84+ archive.owner, spn, archive, component)
85+
86
87 def test_suite():
88 return unittest.TestLoader().loadTestsFromName(__name__)
89
90=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
91--- lib/lp/archiveuploader/uploadprocessor.py 2009-06-24 23:33:29 +0000
92+++ lib/lp/archiveuploader/uploadprocessor.py 2010-01-15 01:34:29 +0000
93@@ -587,9 +587,6 @@
94 # Upload path does not match anything we support.
95 raise UploadPathError("Path format mismatch.")
96
97- if not archive.enabled:
98- raise PPAUploadPathError("%s is disabled." % archive.displayname)
99-
100 if archive.distribution != distribution:
101 raise PPAUploadPathError(
102 "%s only supports uploads to '%s' distribution."
103
104=== modified file 'lib/lp/soyuz/interfaces/sourcepackagerecipe.py'
105--- lib/lp/soyuz/interfaces/sourcepackagerecipe.py 2010-01-15 01:34:24 +0000
106+++ lib/lp/soyuz/interfaces/sourcepackagerecipe.py 2010-01-15 01:34:29 +0000
107@@ -77,6 +77,16 @@
108 def getReferencedBranches():
109 """An iterator of the branches referenced by this recipe."""
110
111+ def requestBuild(self, archive, distroseries, requester, pocket):
112+ """Request that the recipe be built in to the specified archive.
113+
114+ :param archive: The IArchive which you want the build to end up in.
115+ :param distroseries: The DistroSeries that the package should be
116+ targetted to.
117+ :param requester: the person requesting the build.
118+ :param pocket: the pocket that should be targetted.
119+ """
120+
121
122 class ISourcePackageRecipeSource(Interface):
123 """A utility of this interface can be used to create and access recipes.
124
125=== modified file 'lib/lp/soyuz/model/sourcepackagerecipe.py'
126--- lib/lp/soyuz/model/sourcepackagerecipe.py 2010-01-15 01:34:24 +0000
127+++ lib/lp/soyuz/model/sourcepackagerecipe.py 2010-01-15 01:34:29 +0000
128@@ -10,16 +10,26 @@
129
130 from storm.locals import Int, Reference, Store, Storm, Unicode
131
132+from zope.component import getUtility
133 from zope.interface import classProvides, implements
134
135 from canonical.database.datetimecol import UtcDateTimeCol
136 from canonical.launchpad.interfaces.lpstorm import IMasterStore
137
138+from lp.soyuz.interfaces.archive import ArchivePurpose
139+from lp.soyuz.interfaces.component import IComponentSet
140+from lp.archiveuploader.permission import check_upload_to_archive
141 from lp.soyuz.interfaces.sourcepackagerecipe import (
142 ISourcePackageRecipe, ISourcePackageRecipeSource)
143+from lp.soyuz.interfaces.sourcepackagerecipebuild import (
144+ ISourcePackageRecipeBuildSource)
145 from lp.soyuz.model.sourcepackagerecipedata import _SourcePackageRecipeData
146
147
148+class NonPPABuildRequest(Exception):
149+ """A build was requested to a non-PPA and this is currently unsupported."""
150+
151+
152 class SourcePackageRecipe(Storm):
153 """See `ISourcePackageRecipe` and `ISourcePackageRecipeSource`."""
154
155@@ -82,3 +92,19 @@
156 sprecipe.name = name
157 store.add(sprecipe)
158 return sprecipe
159+
160+ def requestBuild(self, archive, requester, pocket):
161+ if archive.purpose != ArchivePurpose.PPA:
162+ raise NonPPABuildRequest
163+ component = getUtility(IComponentSet)["multiverse"]
164+ reject_reason = check_upload_to_archive(requester,
165+ self.distroseries, self.sourcepackagename,
166+ archive, component, pocket)
167+ if reject_reason is not None:
168+ raise reject_reason
169+ sourcepackage = self.distroseries.getSourcePackage(
170+ self.sourcepackagename)
171+ build = getUtility(ISourcePackageRecipeBuildSource).new(sourcepackage,
172+ self, requester, archive)
173+ build.createBuildQueueEntry()
174+ return build
175
176=== modified file 'lib/lp/soyuz/tests/test_sourcepackagerecipe.py'
177--- lib/lp/soyuz/tests/test_sourcepackagerecipe.py 2010-01-15 01:34:24 +0000
178+++ lib/lp/soyuz/tests/test_sourcepackagerecipe.py 2010-01-15 01:34:29 +0000
179@@ -10,14 +10,33 @@
180
181 from bzrlib.plugins.builder.recipe import RecipeParser
182
183+from storm.locals import Store
184+
185 from zope.component import getUtility
186 from zope.security.interfaces import Unauthorized
187+from zope.security.proxy import removeSecurityProxy
188
189 from canonical.testing.layers import DatabaseFunctionalLayer
190
191+from lp.archiveuploader.permission import (
192+ ArchiveDisabled, CannotUploadToArchive, InvalidPocketForPPA)
193+from lp.registry.interfaces.pocket import PackagePublishingPocket
194+from lp.services.job.interfaces.job import (
195+ IJob, JobStatus)
196+from lp.soyuz.interfaces.archive import ArchivePurpose
197+from lp.soyuz.interfaces.buildqueue import (
198+ IBuildQueue)
199 from lp.soyuz.interfaces.sourcepackagerecipe import (
200 ForbiddenInstruction, ISourcePackageRecipe, ISourcePackageRecipeSource,
201 TooNewRecipeFormat)
202+from lp.soyuz.interfaces.sourcepackagerecipebuild import (
203+ ISourcePackageRecipeBuild, ISourcePackageRecipeBuildJob)
204+from lp.soyuz.model.buildqueue import (
205+ BuildQueue)
206+from lp.soyuz.model.sourcepackagerecipebuild import (
207+ SourcePackageRecipeBuildJob)
208+from lp.soyuz.model.sourcepackagerecipe import (
209+ NonPPABuildRequest)
210 from lp.testing import login_person, TestCaseWithFactory
211
212
213@@ -154,6 +173,56 @@
214 TooNewRecipeFormat,
215 self.makeSourcePackageRecipeFromBuilderRecipe, builder_recipe)
216
217+ def test_requestBuild(self):
218+ recipe = self.factory.makeSourcePackageRecipe()
219+ ppa = self.factory.makeArchive()
220+ build = recipe.requestBuild(ppa, ppa.owner,
221+ PackagePublishingPocket.RELEASE)
222+ # TODO: Fails as SourcePackageRecipeBuild doesn't correctly
223+ # implement the interface currently.
224+ #self.assertProvides(build, ISourcePackageRecipeBuild)
225+ self.assertEqual(build.archive, ppa)
226+ self.assertEqual(build.distroseries, recipe.distroseries)
227+ self.assertEqual(build.requester, ppa.owner)
228+ store = Store.of(build)
229+ store.flush()
230+ build_job = store.find(SourcePackageRecipeBuildJob,
231+ SourcePackageRecipeBuildJob.build_id==build.id).one()
232+ self.assertProvides(build_job, ISourcePackageRecipeBuildJob)
233+ self.assertTrue(build_job.virtualized)
234+ job = build_job.job
235+ self.assertProvides(job, IJob)
236+ self.assertEquals(job.status, JobStatus.WAITING)
237+ build_queue = store.find(BuildQueue, BuildQueue.job==job.id).one()
238+ self.assertProvides(build_queue, IBuildQueue)
239+ self.assertTrue(build_queue.virtualized)
240+
241+ def test_requestBuildRejectsNotPPA(self):
242+ recipe = self.factory.makeSourcePackageRecipe()
243+ not_ppa = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
244+ self.assertRaises(NonPPABuildRequest, recipe.requestBuild, not_ppa,
245+ not_ppa.owner, PackagePublishingPocket.RELEASE)
246+
247+ def test_requestBuildRejectsNoPermission(self):
248+ recipe = self.factory.makeSourcePackageRecipe()
249+ ppa = self.factory.makeArchive()
250+ requester = self.factory.makePerson()
251+ self.assertRaises(CannotUploadToArchive, recipe.requestBuild, ppa,
252+ requester, PackagePublishingPocket.RELEASE)
253+
254+ def test_requestBuildRejectsInvalidPocket(self):
255+ recipe = self.factory.makeSourcePackageRecipe()
256+ ppa = self.factory.makeArchive()
257+ self.assertRaises(InvalidPocketForPPA, recipe.requestBuild, ppa,
258+ ppa.owner, PackagePublishingPocket.BACKPORTS)
259+
260+ def test_requestBuildRejectsDisabledArchive(self):
261+ recipe = self.factory.makeSourcePackageRecipe()
262+ ppa = self.factory.makeArchive()
263+ removeSecurityProxy(ppa).disable()
264+ self.assertRaises(ArchiveDisabled, recipe.requestBuild, ppa,
265+ ppa.owner, PackagePublishingPocket.RELEASE)
266+
267
268 class TestRecipeBranchRoundTripping(TestCaseWithFactory):
269