launchpad-reviewers team mailing list archive

[Colin Watson <mp+435978@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:avoid-artifactory-deletion into launchpad:master.

Commit message:
Make publisher ignore pending-deletion Artifactory archives

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/435978

Actually deleting these is unlikely to be at all urgent and can be done manually if needed.  Meanwhile, the previous code was causing the publisher to crash with `NotImplementedError: Don't know how to delete archives published using Artifactory`, which caused anything after that point in the publisher's to-do list not to happen.

I also fixed a couple of existing test assertions that I don't believe were quite doing what the test author intended, since they would have passed regardless of whether the archive in question was deleted.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:avoid-artifactory-deletion into launchpad:master.

diff --git a/lib/lp/archivepublisher/scripts/publishdistro.py b/lib/lp/archivepublisher/scripts/publishdistro.py
index 6c93a6e..3b91fa5 100644
--- a/lib/lp/archivepublisher/scripts/publishdistro.py
+++ b/lib/lp/archivepublisher/scripts/publishdistro.py
@@ -384,7 +384,10 @@ class PublishDistro(PublisherScript):
 
     def deleteArchive(self, archive, publisher):
         """Ask `publisher` to delete `archive`."""
-        if archive.purpose == ArchivePurpose.PPA:
+        if (
+            archive.purpose == ArchivePurpose.PPA
+            and archive.publishing_method == ArchivePublishingMethod.LOCAL
+        ):
             publisher.deleteArchive()
             return True
         else:
diff --git a/lib/lp/archivepublisher/tests/test_publishdistro.py b/lib/lp/archivepublisher/tests/test_publishdistro.py
index f99ee6e..fd1d1e6 100644
--- a/lib/lp/archivepublisher/tests/test_publishdistro.py
+++ b/lib/lp/archivepublisher/tests/test_publishdistro.py
@@ -1109,7 +1109,7 @@ class TestPublishDistroMethods(TestCaseWithFactory):
             ppa, script.getPublisher(distro, ppa, [])
         )
         self.assertTrue(deletion_done)
-        self.assertContentEqual([], script.getPPAs(distro))
+        self.assertEqual(ArchiveStatus.DELETED, ppa.status)
 
     def test_deleteArchive_ignores_non_ppa(self):
         # If fed an archive that's not a PPA, deleteArchive will do
@@ -1121,7 +1121,21 @@ class TestPublishDistroMethods(TestCaseWithFactory):
         script = self.makeScript(distro)
         deletion_done = script.deleteArchive(archive, None)
         self.assertFalse(deletion_done)
-        self.assertEqual(archive, distro.getArchiveByComponent("partner"))
+        self.assertEqual(ArchiveStatus.ACTIVE, archive.status)
+
+    def test_deleteArchive_ignores_non_local(self):
+        # If fed an Artifactory PPA, deleteArchive will do nothing and
+        # return False to indicate the fact.
+        distro = self.makeDistro()
+        ppa = self.factory.makeArchive(
+            distro,
+            purpose=ArchivePurpose.PPA,
+            publishing_method=ArchivePublishingMethod.ARTIFACTORY,
+        )
+        script = self.makeScript(distro)
+        deletion_done = script.deleteArchive(ppa, None)
+        self.assertFalse(deletion_done)
+        self.assertEqual(ArchiveStatus.ACTIVE, ppa.status)
 
     def test_publishArchive_drives_publisher(self):
         # publishArchive puts a publisher through its paces.  This work