Merge lp:~julian-edwards/launchpad/cancel-build-bug-173018-ui-part3 into lp:launchpad

Proposed by Julian Edwards on 2011-11-01
Status: Merged
Approved by: Julian Edwards on 2011-11-01
Approved revision: 14222
Merged at revision: 14224
Proposed branch: lp:~julian-edwards/launchpad/cancel-build-bug-173018-ui-part3
Merge into: lp:launchpad
Diff against target: 269 lines (+170/-3) 4 files modified
To merge this branch: bzr merge lp:~julian-edwards/launchpad/cancel-build-bug-173018-ui-part3
Reviewer Review Type Date Requested Status
Graham Binns code 2011-11-01 Approve on 2011-11-01
Review via email: mp+80888@code.launchpad.net

Commit Message

[r=gmb][bug=173018][incr] Add API functionality to cancel virtual builds.

Description of the Change

Add BinaryPackageBuild.cancel() and can_be_cancelled property, and export both to the webservice.

To post a comment you must log in.
Graham Binns (gmb) wrote :

Hi Julian,

Nice branch, just the one comment, otherwise r=me

186 + def test_cancel_not_in_progress(self):
187 + # Testing the cancel() method for a pending build.

195 + def test_cancel_in_progress(self):
196 + # Testing the cancel() method for a building build.

E_LAZY_DEVELOPER: I can figure out what you're testing from the method names - the comments more-or-less repeat that information - but I don't know what the expected behaviour is unless I read the code. Please update the comments so that they tell me what _should_ happen.

review: Approve (code)
14224. By Julian Edwards on 2011-11-01

suggestion from gmb in review

Preview Diff

1=== modified file 'lib/lp/soyuz/interfaces/binarypackagebuild.py'
2--- lib/lp/soyuz/interfaces/binarypackagebuild.py 2011-06-16 20:12:00 +0000
3+++ lib/lp/soyuz/interfaces/binarypackagebuild.py 2011-11-01 14:12:30 +0000
4@@ -27,6 +27,7 @@
5 export_as_webservice_entry,
6 export_write_operation,
7 exported,
8+ operation_for_version,
9 operation_parameters,
10 )
11 from lazr.restful.fields import Reference
12@@ -119,6 +120,12 @@
13 description=_(
14 "Whether or not this build record can be retried.")))
15
16+ can_be_cancelled = exported(
17+ Bool(
18+ title=_("Can Be Cancelled"), required=False, readonly=True,
19+ description=_(
20+ "Whether or not this build record can be cancelled.")))
21+
22 is_virtualized = Attribute(
23 "Whether or not this build requires a virtual build host or not.")
24
25@@ -223,6 +230,22 @@
26 non-scored BuildQueue entry is created for it.
27 """
28
29+ @export_write_operation()
30+ @operation_for_version("devel")
31+ def cancel():
32+ """Cancel the build if it is either pending or in progress.
33+
34+ Call the can_be_cancelled() method prior to this one to find out if
35+ cancelling the build is possible.
36+
37+ If the build is in progress, it is marked as CANCELLING until the
38+ buildd manager terminates the build and marks it CANCELLED. If the
39+ build is not in progress, it is marked CANCELLED immediately and is
40+ removed from the build queue.
41+
42+ If the build is not in a cancellable state, this method is a no-op.
43+ """
44+
45
46 class IBinaryPackageBuildAdmin(Interface):
47 """A Build interface for items requiring launchpad.Admin."""
48
49=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
50--- lib/lp/soyuz/model/binarypackagebuild.py 2011-09-23 15:46:11 +0000
51+++ lib/lp/soyuz/model/binarypackagebuild.py 2011-11-01 14:12:30 +0000
52@@ -349,6 +349,20 @@
53 """See `IBuild`."""
54 return self.status is BuildStatus.NEEDSBUILD
55
56+ @property
57+ def can_be_cancelled(self):
58+ """See `IBuild`."""
59+ if not self.buildqueue_record:
60+ return False
61+ if self.buildqueue_record.virtualized is False:
62+ return False
63+
64+ cancellable_statuses = [
65+ BuildStatus.BUILDING,
66+ BuildStatus.NEEDSBUILD,
67+ ]
68+ return self.status in cancellable_statuses
69+
70 def retry(self):
71 """See `IBuild`."""
72 assert self.can_be_retried, "Build %s cannot be retried" % self.id
73@@ -377,6 +391,20 @@
74 else:
75 return self.buildqueue_record.lastscore
76
77+ def cancel(self):
78+ """See `IBinaryPackageBuild`."""
79+ if not self.can_be_cancelled:
80+ return
81+
82+ # If the build is currently building we need to tell the
83+ # buildd-manager to terminate it.
84+ if self.status == BuildStatus.BUILDING:
85+ self.status = BuildStatus.CANCELLING
86+ return
87+
88+ # Otherwise we can cancel it here.
89+ self.buildqueue_record.cancel()
90+
91 def makeJob(self):
92 """See `IBuildFarmJob`."""
93 store = Store.of(self)
94
95=== modified file 'lib/lp/soyuz/stories/webservice/xx-builds.txt'
96--- lib/lp/soyuz/stories/webservice/xx-builds.txt 2011-05-04 02:45:25 +0000
97+++ lib/lp/soyuz/stories/webservice/xx-builds.txt 2011-11-01 14:12:30 +0000
98@@ -42,6 +42,7 @@
99 >>> pprint_entry(builds['entries'][0])
100 arch_tag: u'i386'
101 archive_link: u'http://.../beta/~cprov/+archive/ppa'
102+ can_be_cancelled: False
103 can_be_rescored: False
104 can_be_retried: True
105 changesfile_url: None
106@@ -71,6 +72,7 @@
107 archive_link: u'http://.../~cprov/+archive/ppa'
108 build_log_url: u'http://.../~cprov/+archive/ppa/+build/26/+files/netapplet-1.0.0.tar.gz'
109 buildstate: u'Failed to build'
110+ can_be_cancelled: False
111 can_be_rescored: False
112 can_be_retried: True
113 changesfile_url: None
114
115=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
116--- lib/lp/soyuz/tests/test_binarypackagebuild.py 2011-05-04 06:37:50 +0000
117+++ lib/lp/soyuz/tests/test_binarypackagebuild.py 2011-11-01 14:12:30 +0000
118@@ -15,7 +15,15 @@
119
120 from twisted.trial.unittest import TestCase as TrialTestCase
121
122-from canonical.testing.layers import LaunchpadZopelessLayer
123+from canonical.launchpad.testing.pages import (
124+ webservice_for_person,
125+ )
126+from canonical.launchpad.webapp.interaction import ANONYMOUS
127+from canonical.launchpad.webapp.interfaces import OAuthPermission
128+from canonical.testing.layers import (
129+ DatabaseFunctionalLayer,
130+ LaunchpadZopelessLayer,
131+ )
132 from lp.buildmaster.enums import BuildStatus
133 from lp.buildmaster.interfaces.builder import IBuilderSet
134 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
135@@ -27,7 +35,10 @@
136 TestHandleStatusMixin,
137 )
138 from lp.services.job.model.job import Job
139-from lp.soyuz.enums import PackagePublishingStatus
140+from lp.soyuz.enums import (
141+ ArchivePurpose,
142+ PackagePublishingStatus,
143+ )
144 from lp.soyuz.interfaces.binarypackagebuild import (
145 IBinaryPackageBuild,
146 IBinaryPackageBuildSet,
147@@ -39,7 +50,12 @@
148 from lp.soyuz.model.buildpackagejob import BuildPackageJob
149 from lp.soyuz.model.processor import ProcessorFamilySet
150 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
151-from lp.testing import TestCaseWithFactory
152+from lp.testing import (
153+ api_url,
154+ login,
155+ logout,
156+ TestCaseWithFactory,
157+ )
158
159
160 class TestBinaryPackageBuild(TestCaseWithFactory):
161@@ -170,6 +186,48 @@
162 self.assertEquals("Somebody <somebody@ubuntu.com>",
163 self.build.getUploader(MockChanges()))
164
165+ def test_can_be_cancelled(self):
166+ # For all states that can be cancelled, assert can_be_cancelled
167+ # returns True.
168+ ok_cases = [
169+ BuildStatus.BUILDING,
170+ BuildStatus.NEEDSBUILD,
171+ ]
172+ for status in BuildStatus:
173+ if status in ok_cases:
174+ self.assertTrue(self.build.can_be_cancelled)
175+ else:
176+ self.assertFalse(self.build.can_be_cancelled)
177+
178+ def test_can_be_cancelled_virtuality(self):
179+ # Only virtual builds can be cancelled.
180+ bq = removeSecurityProxy(self.build.queueBuild())
181+ bq.virtualized = True
182+ self.assertTrue(self.build.can_be_cancelled)
183+ bq.virtualized = False
184+ self.assertFalse(self.build.can_be_cancelled)
185+
186+ def test_cancel_not_in_progress(self):
187+ # Testing the cancel() method for a pending build should leave
188+ # it in the CANCELLED state.
189+ ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
190+ build = self.factory.makeBinaryPackageBuild(archive=ppa)
191+ build.queueBuild()
192+ build.cancel()
193+ self.assertEqual(BuildStatus.CANCELLED, build.status)
194+ self.assertIs(None, build.buildqueue_record)
195+
196+ def test_cancel_in_progress(self):
197+ # Testing the cancel() method for a building build should leave
198+ # it in the CANCELLING state.
199+ ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
200+ build = self.factory.makeBinaryPackageBuild(archive=ppa)
201+ bq = build.queueBuild()
202+ build.status = BuildStatus.BUILDING
203+ build.cancel()
204+ self.assertEqual(BuildStatus.CANCELLING, build.status)
205+ self.assertEqual(bq, build.buildqueue_record)
206+
207
208 class TestBuildUpdateDependencies(TestCaseWithFactory):
209
210@@ -503,3 +561,59 @@
211 class TestHandleStatusForBinaryPackageBuild(
212 MakeBinaryPackageBuildMixin, TestHandleStatusMixin, TrialTestCase):
213 """IPackageBuild.handleStatus works with binary builds."""
214+
215+
216+class TestBinaryPackageBuildWebservice(TestCaseWithFactory):
217+ """Test cases for BinaryPackageBuild on the webservice.
218+
219+ NB. Note that most tests are currently in
220+ lib/lp/soyuz/stories/webservice/xx-builds.txt but unit tests really
221+ ought to be here instead.
222+ """
223+
224+ layer = DatabaseFunctionalLayer
225+
226+ def setUp(self):
227+ super(TestBinaryPackageBuildWebservice, self).setUp()
228+ self.ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
229+ self.build = self.factory.makeBinaryPackageBuild(archive=self.ppa)
230+ self.webservice = webservice_for_person(
231+ self.ppa.owner, permission=OAuthPermission.WRITE_PUBLIC)
232+ login(ANONYMOUS)
233+
234+ def test_can_be_cancelled_is_exported(self):
235+ # Check that the can_be_cancelled property is exported.
236+ expected = self.build.can_be_cancelled
237+ entry_url = api_url(self.build)
238+ logout()
239+ entry = self.webservice.get(
240+ entry_url, api_version='devel').jsonBody()
241+ self.assertEqual(expected, entry['can_be_cancelled'])
242+
243+ def test_cancel_is_exported(self):
244+ # Check that the cancel() named op is exported.
245+ build_url = api_url(self.build)
246+ self.build.queueBuild()
247+ logout()
248+ entry = self.webservice.get(
249+ build_url, api_version='devel').jsonBody()
250+ response = self.webservice.named_post(
251+ entry['self_link'], 'cancel', api_version='devel')
252+ self.assertEqual(200, response.status)
253+ entry = self.webservice.get(
254+ build_url, api_version='devel').jsonBody()
255+ self.assertEqual(BuildStatus.CANCELLED.title, entry['buildstate'])
256+
257+ def test_cancel_security(self):
258+ # Check that unauthorised users cannot call cancel()
259+ build_url = api_url(self.build)
260+ person = self.factory.makePerson()
261+ webservice = webservice_for_person(
262+ person, permission=OAuthPermission.WRITE_PUBLIC)
263+ logout()
264+
265+ entry = webservice.get(
266+ build_url, api_version='devel').jsonBody()
267+ response = webservice.named_post(
268+ entry['self_link'], 'cancel', api_version='devel')
269+ self.assertEqual(401, response.status)