launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19340
[Merge] lp:~wgrant/launchpad/recipe-cancel into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/recipe-cancel into lp:launchpad.
Commit message:
Allow archive owners to cancel their recipe builds, and make recipes use the normal cancellation infrastructure.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #624630 in Launchpad itself: "Canceling a "request pending" recipe not possible"
https://bugs.launchpad.net/launchpad/+bug/624630
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/recipe-cancel/+merge/270810
Allow archive owners to cancel their recipe builds, and make recipes use the normal cancellation infrastructure rather than just setting themselves to SUPERSEDED.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/recipe-cancel into lp:launchpad.
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2015-06-25 04:42:48 +0000
+++ lib/lp/code/browser/configure.zcml 2015-09-11 12:41:23 +0000
@@ -1171,7 +1171,7 @@
class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildCancelView"
name="+cancel"
template="../../app/templates/generic-edit.pt"
- permission="launchpad.Admin"/>
+ permission="launchpad.Edit"/>
<browser:page
for="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuild"
class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildRescoreView"
=== modified file 'lib/lp/code/browser/sourcepackagerecipebuild.py'
--- lib/lp/code/browser/sourcepackagerecipebuild.py 2013-11-18 07:51:18 +0000
+++ lib/lp/code/browser/sourcepackagerecipebuild.py 2015-09-11 12:41:23 +0000
@@ -39,13 +39,6 @@
)
-UNEDITABLE_BUILD_STATES = (
- BuildStatus.FULLYBUILT,
- BuildStatus.FAILEDTOBUILD,
- BuildStatus.SUPERSEDED,
- BuildStatus.FAILEDTOUPLOAD,)
-
-
class SourcePackageRecipeBuildNavigation(Navigation, FileNavigationMixin):
usedfor = ISourcePackageRecipeBuild
@@ -60,21 +53,17 @@
links = ('cancel', 'rescore')
- @enabled_with_permission('launchpad.Admin')
+ @enabled_with_permission('launchpad.Edit')
def cancel(self):
- if self.context.status in UNEDITABLE_BUILD_STATES:
- enabled = False
- else:
- enabled = True
- return Link('+cancel', 'Cancel build', icon='remove', enabled=enabled)
+ return Link(
+ '+cancel', 'Cancel build', icon='remove',
+ enabled=self.context.can_be_cancelled)
@enabled_with_permission('launchpad.Admin')
def rescore(self):
- if self.context.status in UNEDITABLE_BUILD_STATES:
- enabled = False
- else:
- enabled = True
- return Link('+rescore', 'Rescore build', icon='edit', enabled=enabled)
+ return Link(
+ '+rescore', 'Rescore build', icon='edit',
+ enabled=self.context.can_be_rescored)
class SourcePackageRecipeBuildView(LaunchpadView):
@@ -152,7 +141,7 @@
@action('Cancel build', name='cancel')
def request_action(self, action, data):
"""Cancel the build."""
- self.context.cancelBuild()
+ self.context.cancel()
class SourcePackageRecipeBuildRescoreView(LaunchpadFormView):
=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py 2015-08-03 12:59:18 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py 2015-09-11 12:41:23 +0000
@@ -22,7 +22,6 @@
BrowserTestCase,
login,
logout,
- person_logged_in,
TestCaseWithFactory,
)
from lp.testing.layers import DatabaseFunctionalLayer
@@ -83,17 +82,19 @@
daily_build_archive=self.ppa)
build = self.factory.makeSourcePackageRecipeBuild(
recipe=recipe)
+ build.queueBuild()
return build
def test_cancel_build(self):
- """An admin can cancel a build."""
+ """The archive owner can cancel a build."""
queue = self.factory.makeSourcePackageRecipeBuild().queueBuild()
build = queue.specific_build
transaction.commit()
build_url = canonical_url(build)
+ owner = build.archive.owner
logout()
- browser = self.getUserBrowser(build_url, user=self.admin)
+ browser = self.getUserBrowser(build_url, user=owner)
browser.getLink('Cancel build').click()
self.assertEqual(
@@ -107,12 +108,10 @@
build_url)
login(ANONYMOUS)
- self.assertEqual(
- BuildStatus.SUPERSEDED,
- build.status)
+ self.assertEqual(BuildStatus.CANCELLED, build.status)
def test_cancel_build_not_admin(self):
- """No one but an admin can cancel a build."""
+ """A normal user can't cancel a build."""
queue = self.factory.makeSourcePackageRecipeBuild().queueBuild()
build = queue.specific_build
transaction.commit()
@@ -131,12 +130,14 @@
def test_cancel_build_wrong_state(self):
"""If the build isn't queued, you can't cancel it."""
build = self.makeRecipeBuild()
- build.cancelBuild()
+ with admin_logged_in():
+ build.cancel()
transaction.commit()
build_url = canonical_url(build)
+ owner = build.archive.owner
logout()
- browser = self.getUserBrowser(build_url, user=self.admin)
+ browser = self.getUserBrowser(build_url, user=owner)
self.assertRaises(
LinkNotFoundError,
browser.getLink, 'Cancel build')
@@ -212,7 +213,8 @@
def test_rescore_build_wrong_state(self):
"""If the build isn't queued, you can't rescore it."""
build = self.makeRecipeBuild()
- build.cancelBuild()
+ with admin_logged_in():
+ build.cancel()
transaction.commit()
build_url = canonical_url(build)
logout()
@@ -228,25 +230,11 @@
This is the case where the user has a stale link that they click on.
"""
build = self.factory.makeSourcePackageRecipeBuild()
- build.cancelBuild()
- index_url = canonical_url(build)
- browser = self.getViewBrowser(build, '+rescore', user=self.admin)
- self.assertEqual(index_url, browser.url)
- self.assertIn(
- 'Cannot rescore this build because it is not queued.',
- browser.contents)
-
- def test_rescore_build_wrong_state_stale_page(self):
- """Show sane error if you attempt to rescore a non-queued build.
-
- This is the case where the user is on the rescore page and submits.
- """
- build = self.factory.makeSourcePackageRecipeBuild()
- index_url = canonical_url(build)
- browser = self.getViewBrowser(build, '+rescore', user=self.admin)
- with person_logged_in(self.admin):
- build.cancelBuild()
- browser.getLink('Rescore build').click()
+ build.queueBuild()
+ with admin_logged_in():
+ build.cancel()
+ index_url = canonical_url(build)
+ browser = self.getViewBrowser(build, '+rescore', user=self.admin)
self.assertEqual(index_url, browser.url)
self.assertIn(
'Cannot rescore this build because it is not queued.',
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2015-09-01 17:10:46 +0000
+++ lib/lp/code/configure.zcml 2015-09-11 12:41:23 +0000
@@ -1015,7 +1015,12 @@
<class
class="lp.code.model.sourcepackagerecipebuild.SourcePackageRecipeBuild">
- <require permission="launchpad.View" interface="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuild"/>
+ <require
+ permission="launchpad.View"
+ interface="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuildView"/>
+ <require
+ permission="launchpad.Edit"
+ interface="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuildEdit"/>
</class>
<securedutility
=== modified file 'lib/lp/code/interfaces/sourcepackagerecipebuild.py'
--- lib/lp/code/interfaces/sourcepackagerecipebuild.py 2015-05-14 08:50:41 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipebuild.py 2015-09-11 12:41:23 +0000
@@ -9,11 +9,15 @@
'ISourcePackageRecipeBuildSource',
]
-from lazr.restful.declarations import export_as_webservice_entry
+from lazr.restful.declarations import (
+ export_as_webservice_entry,
+ exported,
+ )
from lazr.restful.fields import (
CollectionField,
Reference,
)
+from zope.interface import Interface
from zope.schema import (
Bool,
Int,
@@ -35,9 +39,7 @@
from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease
-class ISourcePackageRecipeBuild(IPackageBuild):
- """A build of a source package."""
- export_as_webservice_entry()
+class ISourcePackageRecipeBuildView(IPackageBuild):
id = Int(title=_("Identifier for this build."))
@@ -56,6 +58,16 @@
recipe = Object(
schema=ISourcePackageRecipe, title=_("The recipe being built."))
+ can_be_rescored = exported(Bool(
+ title=_("Can be rescored"),
+ required=True, readonly=True,
+ description=_("Whether this build record can be rescored manually.")))
+
+ can_be_cancelled = exported(Bool(
+ title=_("Can be cancelled"),
+ required=True, readonly=True,
+ description=_("Whether this build record can be cancelled.")))
+
manifest = Object(
schema=ISourcePackageRecipeData, title=_(
'A snapshot of the recipe for this build.'))
@@ -70,13 +82,34 @@
def getFileByName(filename):
"""Return the file under +files with specified name."""
- def cancelBuild():
- """Cancel the build."""
+
+class ISourcePackageRecipeBuildEdit(Interface):
+
+ def cancel():
+ """Cancel the build if it is either pending or in progress.
+
+ Check the can_be_cancelled property prior to calling this method to
+ find out if cancelling the build is possible.
+
+ If the build is in progress, it is marked as CANCELLING until the
+ buildd manager terminates the build and marks it CANCELLED. If the
+ build is not in progress, it is marked CANCELLED immediately and is
+ removed from the build queue.
+
+ If the build is not in a cancellable state, this method is a no-op.
+ """
def destroySelf():
"""Delete the build itself."""
+class ISourcePackageRecipeBuild(ISourcePackageRecipeBuildView,
+ ISourcePackageRecipeBuildEdit):
+ """A build of a source package."""
+
+ export_as_webservice_entry()
+
+
class ISourcePackageRecipeBuildSource(ISpecificBuildFarmJobSource):
"""A utility of this interface be used to create source package builds."""
=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py 2015-07-08 16:05:11 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py 2015-09-11 12:41:23 +0000
@@ -258,11 +258,31 @@
builds.append(build)
return builds
- def cancelBuild(self):
- """See `ISourcePackageRecipeBuild.`"""
- self.updateStatus(BuildStatus.SUPERSEDED)
- if self.buildqueue_record is not None:
- self.buildqueue_record.destroySelf()
+ @property
+ def can_be_rescored(self):
+ """See `IBuild`."""
+ return self.status is BuildStatus.NEEDSBUILD
+
+ @property
+ def can_be_cancelled(self):
+ """See `ISourcePackageRecipeBuild`."""
+ if not self.buildqueue_record:
+ return False
+
+ cancellable_statuses = [
+ BuildStatus.BUILDING,
+ BuildStatus.NEEDSBUILD,
+ ]
+ return self.status in cancellable_statuses
+
+ def cancel(self):
+ """See `ISourcePackageRecipeBuild`."""
+ if not self.can_be_cancelled:
+ return
+ # BuildQueue.cancel() will decide whether to go straight to
+ # CANCELLED, or go through CANCELLING to let buildd-manager
+ # clean up the slave.
+ self.buildqueue_record.cancel()
def destroySelf(self):
if self.buildqueue_record is not None:
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py 2015-04-30 01:45:30 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py 2015-09-11 12:41:23 +0000
@@ -666,7 +666,8 @@
# Cancelled builds are not considered pending.
recipe = self.factory.makeSourcePackageRecipe()
build = self.factory.makeSourcePackageRecipeBuild(recipe=recipe)
- build.cancelBuild()
+ build.queueBuild()
+ build.cancel()
self.assertEqual([build], list(recipe.builds))
self.assertEqual([build], list(recipe.completed_builds))
self.assertEqual([], list(recipe.pending_builds))
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2015-05-13 08:27:03 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2015-09-11 12:41:23 +0000
@@ -38,6 +38,7 @@
from lp.services.mail.sendmail import format_address
from lp.services.webapp.authorization import check_permission
from lp.testing import (
+ admin_logged_in,
ANONYMOUS,
login,
person_logged_in,
@@ -78,7 +79,8 @@
# SourcePackageRecipeBuild provides IPackageBuild and
# ISourcePackageRecipeBuild.
spb = self.makeSourcePackageRecipeBuild()
- self.assertProvides(spb, ISourcePackageRecipeBuild)
+ with admin_logged_in():
+ self.assertProvides(spb, ISourcePackageRecipeBuild)
def test_implements_interface(self):
build = self.makeSourcePackageRecipeBuild()
@@ -88,7 +90,8 @@
# A source package recipe build can be stored in the database
spb = self.makeSourcePackageRecipeBuild()
transaction.commit()
- self.assertProvides(spb, ISourcePackageRecipeBuild)
+ with admin_logged_in():
+ self.assertProvides(spb, ISourcePackageRecipeBuild)
def test_queueBuild(self):
spb = self.makeSourcePackageRecipeBuild()
@@ -409,7 +412,8 @@
# ISourcePackageRecipeBuild should make sure to remove jobs and build
# queue entries and then invalidate itself.
build = self.factory.makeSourcePackageRecipeBuild()
- build.destroySelf()
+ with admin_logged_in():
+ build.destroySelf()
def test_destroySelf_clears_release(self):
# Destroying a sourcepackagerecipebuild removes references to it from
@@ -418,7 +422,8 @@
release = self.factory.makeSourcePackageRelease(
source_package_recipe_build=build)
self.assertEqual(build, release.source_package_recipe_build)
- build.destroySelf()
+ with admin_logged_in():
+ build.destroySelf()
self.assertIs(None, release.source_package_recipe_build)
transaction.commit()
@@ -431,18 +436,19 @@
# Ensure database ids are set.
store.flush()
build_farm_job_id = naked_build.build_farm_job_id
- build.destroySelf()
+ with admin_logged_in():
+ build.destroySelf()
self.assertIs(None, store.get(BuildFarmJob, build_farm_job_id))
- def test_cancelBuild(self):
+ def test_cancel(self):
# ISourcePackageRecipeBuild should make sure to remove jobs and build
# queue entries and then invalidate itself.
build = self.factory.makeSourcePackageRecipeBuild()
- build.cancelBuild()
+ build.queueBuild()
+ with admin_logged_in():
+ build.cancel()
- self.assertEqual(
- BuildStatus.SUPERSEDED,
- build.status)
+ self.assertEqual(BuildStatus.CANCELLED, build.status)
def test_getUploader(self):
# For ACL purposes the uploader is the build requester.
@@ -522,7 +528,8 @@
build = self.factory.makeSourcePackageRecipeBuild(
recipe=cake, distroseries=secret, archive=pantry)
build.updateStatus(BuildStatus.FULLYBUILT)
- cake.destroySelf()
+ with admin_logged_in():
+ cake.destroySelf()
IStore(build).flush()
build.notify()
notifications = pop_notifications()
Follow ups
