[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad

2016-10-19 Thread noreply 
The proposal to merge lp:~cjwatson/launchpad/snap-delete-with-builds into 
lp:launchpad has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-delete-with-builds/+merge/308767
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad

2016-10-18 Thread Maximiliano Bertacchini 
Review: Approve

Looks good to me.
-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-delete-with-builds/+merge/308767
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad

2016-10-18 Thread Colin Watson 
Colin Watson has proposed merging 
lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad.

Commit message:
Delete associated build jobs when deleting a snap.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1607366 in Launchpad itself: "IntegrityError while attempting to remove 
a snap package with builds"
  https://bugs.launchpad.net/launchpad/+bug/1607366

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-delete-with-builds/+merge/308767
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad.
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2016-07-19 16:32:46 +
+++ lib/lp/snappy/model/snap.py	2016-10-18 21:54:25 +
@@ -480,6 +480,13 @@
 SnapFile.snapbuild = SnapBuild.id AND
 SnapBuild.snap = ?
 """, (self.id,))
+store.execute("""
+DELETE FROM SnapBuildJob
+USING SnapBuild
+WHERE
+SnapBuildJob.snapbuild = SnapBuild.id AND
+SnapBuild.snap = ?
+""", (self.id,))
 store.find(SnapBuild, SnapBuild.snap == self).remove()
 getUtility(IWebhookSet).delete(self.webhooks)
 store.remove(self)

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2016-08-12 12:56:41 +
+++ lib/lp/snappy/tests/test_snap.py	2016-10-18 21:54:25 +
@@ -62,8 +62,10 @@
 ISnapBuild,
 ISnapBuildSet,
 )
+from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 from lp.snappy.model.snap import SnapSet
 from lp.snappy.model.snapbuild import SnapFile
+from lp.snappy.model.snapbuildjob import SnapBuildJob
 from lp.testing import (
 admin_logged_in,
 ANONYMOUS,
@@ -443,7 +445,7 @@
 
 def test_delete_with_builds(self):
 # A snap package with builds can be deleted.  Doing so deletes all
-# its builds and their files too.
+# its builds, their files, and any associated build jobs too.
 owner = self.factory.makePerson()
 distroseries = self.factory.makeDistroSeries()
 snap = self.factory.makeSnap(
@@ -452,6 +454,7 @@
 build = self.factory.makeSnapBuild(snap=snap)
 build_queue = build.queueBuild()
 snapfile = self.factory.makeSnapFile(snapbuild=build)
+snap_build_job = getUtility(ISnapStoreUploadJobSource).create(build)
 self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
 other_build = self.factory.makeSnapBuild()
 other_build.queueBuild()
@@ -460,6 +463,7 @@
 build_id = build.id
 build_queue_id = build_queue.id
 build_farm_job_id = removeSecurityProxy(build).build_farm_job_id
+snap_build_job_id = snap_build_job.job.job_id
 snapfile_id = removeSecurityProxy(snapfile).id
 with person_logged_in(snap.owner):
 snap.destroySelf()
@@ -470,6 +474,7 @@
 self.assertIsNone(store.get(BuildQueue, build_queue_id))
 self.assertIsNone(store.get(BuildFarmJob, build_farm_job_id))
 self.assertIsNone(store.get(SnapFile, snapfile_id))
+self.assertIsNone(store.get(SnapBuildJob, snap_build_job_id))
 # Unrelated builds are still present.
 clear_property_cache(other_build)
 self.assertEqual(

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad

2016-02-28 Thread noreply 
The proposal to merge lp:~cjwatson/launchpad/snap-delete-with-builds into 
lp:launchpad has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-delete-with-builds/+merge/287408
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad

2016-02-28 Thread William Grant 
Review: Approve code


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-delete-with-builds/+merge/287408
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad

2016-02-28 Thread Colin Watson 
Colin Watson has proposed merging 
lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad.

Commit message:
Allow deleting snaps even if they have builds.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-delete-with-builds/+merge/287408

There isn't really a good reason to forbid deleting snaps that have builds.  As 
far as I remember I did that because I was basing that bit of my code on 
recipes, but recipes have extra constraints because recipe builds are linked to 
source packages.  That constraint doesn't exist for snaps, so it should be safe 
to allow this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad.
=== modified file 'lib/lp/snappy/browser/configure.zcml'
--- lib/lp/snappy/browser/configure.zcml	2016-01-27 12:43:00 +
+++ lib/lp/snappy/browser/configure.zcml	2016-02-28 17:16:06 +
@@ -71,7 +71,7 @@
 class="lp.snappy.browser.snap.SnapDeleteView"
 permission="launchpad.Edit"
 name="+delete"
-template="../templates/snap-delete.pt" />
+template="../../app/templates/generic-edit.pt" />
 >> MERGE-SOURCE
 store.remove(self)
 
 

=== removed file 'lib/lp/snappy/templates/snap-delete.pt'
--- lib/lp/snappy/templates/snap-delete.pt	2015-09-04 16:20:26 +
+++ lib/lp/snappy/templates/snap-delete.pt	1970-01-01 00:00:00 +
@@ -1,21 +0,0 @@
-http://www.w3.org/1999/xhtml";
-  xmlns:tal="http://xml.zope.org/namespaces/tal";
-  xmlns:metal="http://xml.zope.org/namespaces/metal";
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n";
-  metal:use-macro="view/macro:page/main_only"
-  i18n:domain="launchpad">
-
-
-  
-
-  This snap package cannot be deleted as builds have been created for it.
-
-
-
-  
-
-  
-
-
-

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2016-02-19 14:18:22 +
+++ lib/lp/snappy/tests/test_snap.py	2016-02-28 17:16:06 +
@@ -32,11 +32,12 @@
 ONE_DAY_AGO,
 UTC_NOW,
 )
+from lp.services.database.interfaces import IMasterStore
+from lp.services.database.sqlbase import flush_database_caches
 from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.interfaces import OAuthPermission
 from lp.snappy.interfaces.snap import (
 BadSnapSearchContext,
-CannotDeleteSnap,
 CannotModifySnapProcessor,
 ISnap,
 ISnapSet,
@@ -50,7 +51,11 @@
 SnapPrivacyMismatch,
 SnapPrivateFeatureDisabled,
 )
-from lp.snappy.interfaces.snapbuild import ISnapBuild
+from lp.snappy.interfaces.snapbuild import (
+ISnapBuild,
+ISnapBuildSet,
+)
+from lp.snappy.model.snapbuild import SnapFile
 from lp.testing import (
 admin_logged_in,
 ANONYMOUS,
@@ -359,18 +364,34 @@
 snap.destroySelf()
 self.assertFalse(getUtility(ISnapSet).exists(owner, u"condemned"))
 
+
+class TestSnapDeleteWithBuilds(TestCaseWithFactory):
+
+layer = LaunchpadFunctionalLayer
+
+def setUp(self):
+super(TestSnapDeleteWithBuilds, self).setUp()
+self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+
 def test_delete_with_builds(self):
-# A snap package with builds cannot be deleted.
+# A snap package with builds can be deleted.  Doing so deletes all
+# its builds and their files too.
 owner = self.factory.makePerson()
 distroseries = self.factory.makeDistroSeries()
 snap = self.factory.makeSnap(
 registrant=owner, owner=owner, distroseries=distroseries,
 name=u"condemned")
-self.factory.makeSnapBuild(snap=snap)
+build = self.factory.makeSnapBuild(snap=snap)
+snapfile = self.factory.makeSnapFile(snapbuild=build)
 self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
+build_id = build.id
+snapfile_id = removeSecurityProxy(snapfile).id
 with person_logged_in(snap.owner):
-self.assertRaises(CannotDeleteSnap, snap.destroySelf)
-self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
+snap.destroySelf()
+flush_database_caches()
+self.assertFalse(getUtility(ISnapSet).exists(owner, u"condemned"))
+self.assertIsNone(getUtility(ISnapBuildSet).getByID(build_id))
+self.assertIsNone(IMasterStore(SnapFile).get(SnapFile, snapfile_id))
 
 def test_related_webhooks_deleted(self):
 owner = self.factory.makePerson()

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp