Merge lp:~julian-edwards/launchpad/delete-ppa-part2 into lp:launchpad/db-devel

[Julian Edwards]

This is a quick branch bloated a bit by all the lint I fixed. It is a fix to
delete the right directory when deleting PPAs.

Consider this tree structure:

PPA_ROOT/
  <ppa_owner_name>/
     ppa1_name/
        file1
        file2
     ppa2_name/

If we want to delete "ppa1_name" What happens right now is that only the files
*under* the ppa1_name directory are removed.

I've fixed it so that the "ppa1_name" directory itself gets removed.

The test_publisher.py file had lots of lint, please excuse the noise. The
meat of the change is in the deleteArchive() method which no longer raises
OOPSes (it's pointless and was not tested) if it can't delete the archive.
This is a valid scenario since an admin may have done it manually if the
deletion was time-critical. It just logs the error and continues.

In the test, I also added an extra assertion that the archive gets disabled
(was missing from before), and an extra deletion step to prove that deleting a
missing repo doesn't blow up.

Cheers

[Abel Deuring (adeuring)]

review: Approve (code)

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2010-04-15 16:20:04 +0000
+++ lib/lp/archivepublisher/publishing.py	2010-04-23 15:16:27 +0000
@@ -37,8 +37,6 @@
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 
 from canonical.librarian.client import LibrarianClient
-from canonical.launchpad.webapp.errorlog import (
-    ErrorReportingUtility, ScriptRequest)
 
 suffixpocket = dict((v, k) for (k, v) in pocketsuffix.items())
 
@@ -610,28 +608,21 @@
         be caught and an OOPS report generated.
         """
 
+        root_dir = os.path.join(
+            self._config.distroroot, self.archive.owner.name,
+            self.archive.name)
+
         self.log.info(
             "Attempting to delete archive '%s/%s' at '%s'." % (
-                self.archive.owner.name, self.archive.name, 
-                self._config.archiveroot))
+                self.archive.owner.name, self.archive.name, root_dir))
 
-        # Attempt to rmdir if the path to the root of the archive exists.
-        if os.path.exists(self._config.archiveroot):
-            try:
-                shutil.rmtree(self._config.archiveroot)
-            except:
-                message = 'Exception while deleting archive root %s' % (
-                    self._config.archiveroot)
-                request = ScriptRequest([('error-explanation', message)])
-                ErrorReportingUtility().raising(sys.exc_info(), request)
-                self.log.error('%s (%s)' % (message, request.oopsid))
-        else:
+        try:
+            shutil.rmtree(root_dir)
+        except (shutil.Error, OSError), e:
             self.log.warning(
-                "Root directory '%s' for archive '%s/%s' does not exist." % (
-                    self._config.archiveroot, self.archive.owner.name, 
-                    self.archive.name))
+                "Failed to delete directory '%s' for archive '%s/%s'\n%s" % (
+                    root_dir, self.archive.owner.name, 
+                    self.archive.name, e))
 
-        # Set archive's status to DELETED.
         self.archive.status = ArchiveStatus.DELETED
-        # Disable publishing for this archive.
         self.archive.publish = False
 
=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2010-04-15 16:18:09 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2010-04-23 15:16:27 +0000
@@ -8,7 +8,6 @@
 
 import bz2
 import gzip
-import hashlib
 import os
 import shutil
 import stat
@@ -37,7 +36,6 @@
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 from lp.archivepublisher.interfaces.archivesigningkey import (
     IArchiveSigningKey)
-from lp.testing import get_lsb_information
 from lp.soyuz.tests.test_publishing import TestNativePublishingBase
 from canonical.zeca.ftests.harness import ZecaTestSetup
 
@@ -110,8 +108,16 @@
             publisher._config.archiveroot, 'test_file'), 'w').close()
 
         publisher.deleteArchive()
-        self.assertFalse(os.path.exists(publisher._config.archiveroot))
+        root_dir = os.path.join(
+            publisher._config.distroroot, test_archive.owner.name,
+            test_archive.name)
+        self.assertFalse(os.path.exists(root_dir))
         self.assertEqual(test_archive.status, ArchiveStatus.DELETED)
+        self.assertEqual(test_archive.publish, False)
+
+        # Trying to delete it again won't fail, in the corner case where
+        # some admin manually deleted the repo.
+        publisher.deleteArchive()
 
     def testPublishPartner(self):
         """Test that a partner package is published to the right place."""
@@ -122,8 +128,7 @@
             pub_config.poolroot, pub_config.temproot, self.logger)
         publisher = Publisher(
             self.logger, pub_config, disk_pool, archive)
-        pub_source = self.getPubSource(archive=archive,
-            filecontent="I am partner")
+        self.getPubSource(archive=archive, filecontent="I am partner")
 
         publisher.A_publish(False)
 
@@ -161,7 +166,7 @@
         disk_pool = DiskPool(
             pub_config.poolroot, pub_config.temproot, self.logger)
         publisher = Publisher(self.logger, pub_config, disk_pool, archive)
-        pub_source = self.getPubSource(
+        self.getPubSource(
             archive=archive, filecontent="I am partner",
             status=PackagePublishingStatus.PENDING)
 
@@ -248,8 +253,7 @@
             self.logger, self.config, self.disk_pool,
             self.ubuntutest.main_archive)
 
-        pub_source = self.getPubSource(
-            status=PackagePublishingStatus.PUBLISHED)
+        self.getPubSource(status=PackagePublishingStatus.PUBLISHED)
 
         # a new non-careful publisher won't find anything to publish, thus
         # no pockets will be *dirtied*.
@@ -269,7 +273,7 @@
             self.logger, self.config, self.disk_pool,
             self.ubuntutest.main_archive)
 
-        pub_source = self.getPubSource(
+        self.getPubSource(
             filecontent='Hello world',
             status=PackagePublishingStatus.PUBLISHED)
 
@@ -412,17 +416,17 @@
         ubuntu = getUtility(IDistributionSet)['ubuntu']
 
         spiv = person_set.getByName('spiv')
-        spiv_archive = archive_set.new(
+        archive_set.new(
             owner=spiv, distribution=ubuntu, purpose=ArchivePurpose.PPA)
         name16 = person_set.getByName('name16')
-        name16_archive = archive_set.new(
+        archive_set.new(
             owner=name16, distribution=ubuntu, purpose=ArchivePurpose.PPA)
 
-        pub_source = self.getPubSource(
+        self.getPubSource(
             sourcename="foo", filename="foo_1.dsc", filecontent='Hello world',
             status=PackagePublishingStatus.PENDING, archive=spiv.archive)
 
-        pub_source = self.getPubSource(
+        self.getPubSource(
             sourcename="foo", filename="foo_1.dsc", filecontent='Hello world',
             status=PackagePublishingStatus.PUBLISHED, archive=name16.archive)
 
@@ -485,7 +489,7 @@
         pub_source = self.getPubSource(
             sourcename="foo", filename="foo_1.dsc", filecontent='Hello world',
             status=PackagePublishingStatus.PENDING, archive=cprov.archive)
-        pub_bin = self.getPubBinaries(
+        self.getPubBinaries(
             pub_source=pub_source,
             description="   My leading spaces are normalised to a single "
                         "space but not trailing.  \n    It does nothing, "
@@ -496,7 +500,7 @@
         ignored_source = self.getPubSource(
             status=PackagePublishingStatus.DELETED,
             archive=cprov.archive)
-        pub_udeb = self.getPubBinaries(
+        self.getPubBinaries(
             pub_source=ignored_source, binaryname='bingo',
             description='nice udeb', format=BinaryPackageFormat.UDEB)[0]
 
@@ -627,27 +631,27 @@
         # waiting to be deleted, each in different pockets.  The deleted
         # source in the release pocket should not be processed.  We'll
         # also have a binary waiting to be deleted.
-        published_source = self.getPubSource(
+        self.getPubSource(
             pocket=PackagePublishingPocket.RELEASE,
             status=PackagePublishingStatus.PUBLISHED)
 
-        deleted_source_in_release_pocket = self.getPubSource(
+        self.getPubSource(
             pocket=PackagePublishingPocket.RELEASE,
             status=PackagePublishingStatus.DELETED)
 
-        removed_source = self.getPubSource(
+        self.getPubSource(
             scheduleddeletiondate=UTC_NOW,
             dateremoved=UTC_NOW,
             pocket=PackagePublishingPocket.UPDATES,
             status=PackagePublishingStatus.DELETED)
 
-        deleted_source = self.getPubSource(
+        self.getPubSource(
             pocket=PackagePublishingPocket.SECURITY,
             status=PackagePublishingStatus.DELETED)
 
-        deleted_binary = self.getPubBinaries(
+        self.getPubBinaries(
             pocket=PackagePublishingPocket.BACKPORTS,
-            status=PackagePublishingStatus.DELETED)[0]
+            status=PackagePublishingStatus.DELETED)
 
         # Run the deletion detection.
         publisher.A2_markPocketsWithDeletionsDirty()
@@ -699,19 +703,19 @@
 
         # Create pending deletions in RELEASE, BACKPORTS, SECURITY and
         # UPDATES pockets.
-        deleted_source = self.getPubSource(
+        self.getPubSource(
             pocket=PackagePublishingPocket.RELEASE,
             status=PackagePublishingStatus.DELETED)
 
-        deleted_binary = self.getPubBinaries(
+        self.getPubBinaries(
             pocket=PackagePublishingPocket.BACKPORTS,
             status=PackagePublishingStatus.DELETED)[0]
 
-        allowed_source_deletion = self.getPubSource(
+        self.getPubSource(
             pocket=PackagePublishingPocket.SECURITY,
             status=PackagePublishingStatus.DELETED)
 
-        allowed_binary_deletion = self.getPubBinaries(
+        self.getPubBinaries(
             pocket=PackagePublishingPocket.UPDATES,
             status=PackagePublishingStatus.DELETED)[0]
 
@@ -781,7 +785,7 @@
             self.logger, self.config, self.disk_pool,
             self.ubuntutest.main_archive)
 
-        pub_source = self.getPubSource(filecontent='Hello world')
+        self.getPubSource(filecontent='Hello world')
 
         publisher.A_publish(False)
         publisher.C_doFTPArchive(False)
@@ -866,8 +870,7 @@
         archive_publisher = getPublisher(
             cprov.archive, allowed_suites, self.logger)
 
-        pub_source = self.getPubSource(
-            filecontent='Hello world', archive=cprov.archive)
+        self.getPubSource(filecontent='Hello world', archive=cprov.archive)
 
         archive_publisher.A_publish(False)
         self.layer.txn.commit()
@@ -970,8 +973,7 @@
         allowed_suites = []
         archive_publisher = getPublisher(
             named_ppa, allowed_suites, self.logger)
-        pub_source = self.getPubSource(
-            filecontent='Hello world', archive=named_ppa)
+        self.getPubSource(filecontent='Hello world', archive=named_ppa)
 
         archive_publisher.A_publish(False)
         self.layer.txn.commit()
@@ -1080,7 +1082,7 @@
         Publish files in pool, generate archive indexes and release files.
         """
         self.setupPublisher(archive)
-        pub_source = self.getPubSource(archive=archive)
+        self.getPubSource(archive=archive)
 
         self.archive_publisher.A_publish(False)
         transaction.commit()