launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/das-removechroot-api into lp:launchpad.

Commit message:
Add IDistroSeries.removeChroot(), allowing owners of the relevant main_archive to remove chroots using the API.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1194175 in Launchpad itself: "Should be possible to remove chroots over the API"
  https://bugs.launchpad.net/launchpad/+bug/1194175

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/das-removechroot-api/+merge/171125

Following up on https://code.launchpad.net/~stevenk/launchpad/das-setchroot-api/+merge/167464, this adds IDistroArchSeries.removeChroot(), which should make it possible to replace all of manage-chroot.py with an API script.
-- 
https://code.launchpad.net/~cjwatson/launchpad/das-removechroot-api/+merge/171125
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/das-removechroot-api into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/tests/test_distroarchseries_webservice.py'
--- lib/lp/soyuz/browser/tests/test_distroarchseries_webservice.py	2013-06-05 06:38:49 +0000
+++ lib/lp/soyuz/browser/tests/test_distroarchseries_webservice.py	2013-06-24 16:00:51 +0000
@@ -58,14 +58,15 @@
         #See note above regarding testing of length of .entries
         self.assertEqual(1, len(ws_distroseries.architectures.entries))
 
-    def test_setChroot_random_user(self):
-        # Random users are not allowed to set chroots.
+    def test_setChroot_removeChroot_random_user(self):
+        # Random users are not allowed to set or remove chroots.
         das = self.factory.makeDistroArchSeries()
         user = self.factory.makePerson()
         webservice = launchpadlib_for("testing", user, version='devel')
         ws_das = ws_object(webservice, das)
         self.assertRaises(
-            Unauthorized, ws_das.setChroot, data='xyz',sha1sum='0')
+            Unauthorized, ws_das.setChroot, data='xyz', sha1sum='0')
+        self.assertRaises(Unauthorized, ws_das.removeChroot)
 
     def test_setChroot_wrong_sha1sum(self):
         # If the sha1sum calculated is different, the chroot is not set.
@@ -76,7 +77,7 @@
         self.assertRaises(
             BadRequest, ws_das.setChroot, data='zyx', sha1sum='x')
 
-    def test_setChroot(self):
+    def test_setChroot_removeChroot(self):
         das = self.factory.makeDistroArchSeries()
         user = das.distroseries.distribution.main_archive.owner
         expected_file = 'chroot-%s-%s-%s.tar.bz2' % (
@@ -87,3 +88,5 @@
         sha1 = hashlib.sha1('abcxyz').hexdigest()
         ws_das.setChroot(data='abcxyz', sha1sum=sha1)
         self.assertTrue(ws_das.chroot_url.endswith(expected_file))
+        ws_das.removeChroot()
+        self.assertIsNone(ws_das.chroot_url)

=== modified file 'lib/lp/soyuz/interfaces/distroarchseries.py'
--- lib/lp/soyuz/interfaces/distroarchseries.py	2013-06-06 06:59:55 +0000
+++ lib/lp/soyuz/interfaces/distroarchseries.py	2013-06-24 16:00:51 +0000
@@ -198,6 +198,7 @@
         this distro arch series.
         """
 
+
 class IDistroArchSeriesModerate(Interface):
 
     @operation_parameters(data=Bytes(), sha1sum=Text())
@@ -205,10 +206,15 @@
     @operation_for_version("devel")
     def setChroot(data, sha1sum):
         """Set the chroot tarball used for builds in this architecture.
-        
+
         The SHA-1 checksum must match the chroot file.
         """
 
+    @export_write_operation()
+    @operation_for_version("devel")
+    def removeChroot():
+        """Remove the chroot tarball used for builds in this architecture."""
+
 
 class IDistroArchSeries(IDistroArchSeriesPublic, IDistroArchSeriesModerate):
     """An architecture for a distroseries."""

=== modified file 'lib/lp/soyuz/model/distroarchseries.py'
--- lib/lp/soyuz/model/distroarchseries.py	2013-06-20 05:50:00 +0000
+++ lib/lp/soyuz/model/distroarchseries.py	2013-06-24 16:00:51 +0000
@@ -189,6 +189,10 @@
             raise InvalidChrootUploaded("Chroot upload checksums do not match")
         self.addOrUpdateChroot(lfa)
 
+    def removeChroot(self):
+        """See `IDistroArchSeries`."""
+        self.addOrUpdateChroot(None)
+
     def searchBinaryPackages(self, text):
         """See `IDistroArchSeries`."""
         from lp.soyuz.model.publishing import BinaryPackagePublishingHistory