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

Proposed by Julian Edwards on 2010-04-22
Status: Merged
Approved by: Julian Edwards on 2010-04-23
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~julian-edwards/launchpad/delete-ppa-part2
Merge into: lp:launchpad/db-devel
Diff against target: 264 lines (+44/-51)
2 files modified
lib/lp/archivepublisher/publishing.py (+11/-20)
lib/lp/archivepublisher/tests/test_publisher.py (+33/-31)
To merge this branch: bzr merge lp:~julian-edwards/launchpad/delete-ppa-part2
Reviewer Review Type Date Requested Status
Abel Deuring (community) code 2010-04-22 Approve on 2010-04-23
Review via email: mp+23967@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Abel Deuring (adeuring) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/archivepublisher/publishing.py'
2--- lib/lp/archivepublisher/publishing.py 2010-04-15 16:20:04 +0000
3+++ lib/lp/archivepublisher/publishing.py 2010-04-23 15:16:27 +0000
4@@ -37,8 +37,6 @@
5 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
6
7 from canonical.librarian.client import LibrarianClient
8-from canonical.launchpad.webapp.errorlog import (
9- ErrorReportingUtility, ScriptRequest)
10
11 suffixpocket = dict((v, k) for (k, v) in pocketsuffix.items())
12
13@@ -610,28 +608,21 @@
14 be caught and an OOPS report generated.
15 """
16
17+ root_dir = os.path.join(
18+ self._config.distroroot, self.archive.owner.name,
19+ self.archive.name)
20+
21 self.log.info(
22 "Attempting to delete archive '%s/%s' at '%s'." % (
23- self.archive.owner.name, self.archive.name,
24- self._config.archiveroot))
25+ self.archive.owner.name, self.archive.name, root_dir))
26
27- # Attempt to rmdir if the path to the root of the archive exists.
28- if os.path.exists(self._config.archiveroot):
29- try:
30- shutil.rmtree(self._config.archiveroot)
31- except:
32- message = 'Exception while deleting archive root %s' % (
33- self._config.archiveroot)
34- request = ScriptRequest([('error-explanation', message)])
35- ErrorReportingUtility().raising(sys.exc_info(), request)
36- self.log.error('%s (%s)' % (message, request.oopsid))
37- else:
38+ try:
39+ shutil.rmtree(root_dir)
40+ except (shutil.Error, OSError), e:
41 self.log.warning(
42- "Root directory '%s' for archive '%s/%s' does not exist." % (
43- self._config.archiveroot, self.archive.owner.name,
44- self.archive.name))
45+ "Failed to delete directory '%s' for archive '%s/%s'\n%s" % (
46+ root_dir, self.archive.owner.name,
47+ self.archive.name, e))
48
49- # Set archive's status to DELETED.
50 self.archive.status = ArchiveStatus.DELETED
51- # Disable publishing for this archive.
52 self.archive.publish = False
53
54=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
55--- lib/lp/archivepublisher/tests/test_publisher.py 2010-04-15 16:18:09 +0000
56+++ lib/lp/archivepublisher/tests/test_publisher.py 2010-04-23 15:16:27 +0000
57@@ -8,7 +8,6 @@
58
59 import bz2
60 import gzip
61-import hashlib
62 import os
63 import shutil
64 import stat
65@@ -37,7 +36,6 @@
66 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
67 from lp.archivepublisher.interfaces.archivesigningkey import (
68 IArchiveSigningKey)
69-from lp.testing import get_lsb_information
70 from lp.soyuz.tests.test_publishing import TestNativePublishingBase
71 from canonical.zeca.ftests.harness import ZecaTestSetup
72
73@@ -110,8 +108,16 @@
74 publisher._config.archiveroot, 'test_file'), 'w').close()
75
76 publisher.deleteArchive()
77- self.assertFalse(os.path.exists(publisher._config.archiveroot))
78+ root_dir = os.path.join(
79+ publisher._config.distroroot, test_archive.owner.name,
80+ test_archive.name)
81+ self.assertFalse(os.path.exists(root_dir))
82 self.assertEqual(test_archive.status, ArchiveStatus.DELETED)
83+ self.assertEqual(test_archive.publish, False)
84+
85+ # Trying to delete it again won't fail, in the corner case where
86+ # some admin manually deleted the repo.
87+ publisher.deleteArchive()
88
89 def testPublishPartner(self):
90 """Test that a partner package is published to the right place."""
91@@ -122,8 +128,7 @@
92 pub_config.poolroot, pub_config.temproot, self.logger)
93 publisher = Publisher(
94 self.logger, pub_config, disk_pool, archive)
95- pub_source = self.getPubSource(archive=archive,
96- filecontent="I am partner")
97+ self.getPubSource(archive=archive, filecontent="I am partner")
98
99 publisher.A_publish(False)
100
101@@ -161,7 +166,7 @@
102 disk_pool = DiskPool(
103 pub_config.poolroot, pub_config.temproot, self.logger)
104 publisher = Publisher(self.logger, pub_config, disk_pool, archive)
105- pub_source = self.getPubSource(
106+ self.getPubSource(
107 archive=archive, filecontent="I am partner",
108 status=PackagePublishingStatus.PENDING)
109
110@@ -248,8 +253,7 @@
111 self.logger, self.config, self.disk_pool,
112 self.ubuntutest.main_archive)
113
114- pub_source = self.getPubSource(
115- status=PackagePublishingStatus.PUBLISHED)
116+ self.getPubSource(status=PackagePublishingStatus.PUBLISHED)
117
118 # a new non-careful publisher won't find anything to publish, thus
119 # no pockets will be *dirtied*.
120@@ -269,7 +273,7 @@
121 self.logger, self.config, self.disk_pool,
122 self.ubuntutest.main_archive)
123
124- pub_source = self.getPubSource(
125+ self.getPubSource(
126 filecontent='Hello world',
127 status=PackagePublishingStatus.PUBLISHED)
128
129@@ -412,17 +416,17 @@
130 ubuntu = getUtility(IDistributionSet)['ubuntu']
131
132 spiv = person_set.getByName('spiv')
133- spiv_archive = archive_set.new(
134+ archive_set.new(
135 owner=spiv, distribution=ubuntu, purpose=ArchivePurpose.PPA)
136 name16 = person_set.getByName('name16')
137- name16_archive = archive_set.new(
138+ archive_set.new(
139 owner=name16, distribution=ubuntu, purpose=ArchivePurpose.PPA)
140
141- pub_source = self.getPubSource(
142+ self.getPubSource(
143 sourcename="foo", filename="foo_1.dsc", filecontent='Hello world',
144 status=PackagePublishingStatus.PENDING, archive=spiv.archive)
145
146- pub_source = self.getPubSource(
147+ self.getPubSource(
148 sourcename="foo", filename="foo_1.dsc", filecontent='Hello world',
149 status=PackagePublishingStatus.PUBLISHED, archive=name16.archive)
150
151@@ -485,7 +489,7 @@
152 pub_source = self.getPubSource(
153 sourcename="foo", filename="foo_1.dsc", filecontent='Hello world',
154 status=PackagePublishingStatus.PENDING, archive=cprov.archive)
155- pub_bin = self.getPubBinaries(
156+ self.getPubBinaries(
157 pub_source=pub_source,
158 description=" My leading spaces are normalised to a single "
159 "space but not trailing. \n It does nothing, "
160@@ -496,7 +500,7 @@
161 ignored_source = self.getPubSource(
162 status=PackagePublishingStatus.DELETED,
163 archive=cprov.archive)
164- pub_udeb = self.getPubBinaries(
165+ self.getPubBinaries(
166 pub_source=ignored_source, binaryname='bingo',
167 description='nice udeb', format=BinaryPackageFormat.UDEB)[0]
168
169@@ -627,27 +631,27 @@
170 # waiting to be deleted, each in different pockets. The deleted
171 # source in the release pocket should not be processed. We'll
172 # also have a binary waiting to be deleted.
173- published_source = self.getPubSource(
174+ self.getPubSource(
175 pocket=PackagePublishingPocket.RELEASE,
176 status=PackagePublishingStatus.PUBLISHED)
177
178- deleted_source_in_release_pocket = self.getPubSource(
179+ self.getPubSource(
180 pocket=PackagePublishingPocket.RELEASE,
181 status=PackagePublishingStatus.DELETED)
182
183- removed_source = self.getPubSource(
184+ self.getPubSource(
185 scheduleddeletiondate=UTC_NOW,
186 dateremoved=UTC_NOW,
187 pocket=PackagePublishingPocket.UPDATES,
188 status=PackagePublishingStatus.DELETED)
189
190- deleted_source = self.getPubSource(
191+ self.getPubSource(
192 pocket=PackagePublishingPocket.SECURITY,
193 status=PackagePublishingStatus.DELETED)
194
195- deleted_binary = self.getPubBinaries(
196+ self.getPubBinaries(
197 pocket=PackagePublishingPocket.BACKPORTS,
198- status=PackagePublishingStatus.DELETED)[0]
199+ status=PackagePublishingStatus.DELETED)
200
201 # Run the deletion detection.
202 publisher.A2_markPocketsWithDeletionsDirty()
203@@ -699,19 +703,19 @@
204
205 # Create pending deletions in RELEASE, BACKPORTS, SECURITY and
206 # UPDATES pockets.
207- deleted_source = self.getPubSource(
208+ self.getPubSource(
209 pocket=PackagePublishingPocket.RELEASE,
210 status=PackagePublishingStatus.DELETED)
211
212- deleted_binary = self.getPubBinaries(
213+ self.getPubBinaries(
214 pocket=PackagePublishingPocket.BACKPORTS,
215 status=PackagePublishingStatus.DELETED)[0]
216
217- allowed_source_deletion = self.getPubSource(
218+ self.getPubSource(
219 pocket=PackagePublishingPocket.SECURITY,
220 status=PackagePublishingStatus.DELETED)
221
222- allowed_binary_deletion = self.getPubBinaries(
223+ self.getPubBinaries(
224 pocket=PackagePublishingPocket.UPDATES,
225 status=PackagePublishingStatus.DELETED)[0]
226
227@@ -781,7 +785,7 @@
228 self.logger, self.config, self.disk_pool,
229 self.ubuntutest.main_archive)
230
231- pub_source = self.getPubSource(filecontent='Hello world')
232+ self.getPubSource(filecontent='Hello world')
233
234 publisher.A_publish(False)
235 publisher.C_doFTPArchive(False)
236@@ -866,8 +870,7 @@
237 archive_publisher = getPublisher(
238 cprov.archive, allowed_suites, self.logger)
239
240- pub_source = self.getPubSource(
241- filecontent='Hello world', archive=cprov.archive)
242+ self.getPubSource(filecontent='Hello world', archive=cprov.archive)
243
244 archive_publisher.A_publish(False)
245 self.layer.txn.commit()
246@@ -970,8 +973,7 @@
247 allowed_suites = []
248 archive_publisher = getPublisher(
249 named_ppa, allowed_suites, self.logger)
250- pub_source = self.getPubSource(
251- filecontent='Hello world', archive=named_ppa)
252+ self.getPubSource(filecontent='Hello world', archive=named_ppa)
253
254 archive_publisher.A_publish(False)
255 self.layer.txn.commit()
256@@ -1080,7 +1082,7 @@
257 Publish files in pool, generate archive indexes and release files.
258 """
259 self.setupPublisher(archive)
260- pub_source = self.getPubSource(archive=archive)
261+ self.getPubSource(archive=archive)
262
263 self.archive_publisher.A_publish(False)
264 transaction.commit()

Subscribers

People subscribed via source and target branches

to status/vote changes: