Merge lp:~cody-somerville/launchpad/ppa-deletion into lp:launchpad/db-devel
- ppa-deletion
- Merge into db-devel
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Julian Edwards | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | not available | ||||
Proposed branch: | lp:~cody-somerville/launchpad/ppa-deletion | ||||
Merge into: | lp:launchpad/db-devel | ||||
Diff against target: |
272 lines (+142/-23) 5 files modified
lib/lp/archivepublisher/publishing.py (+40/-1) lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py (+8/-1) lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py (+42/-0) lib/lp/archivepublisher/tests/test_publisher.py (+19/-1) lib/lp/soyuz/scripts/publishdistro.py (+33/-20) |
||||
To merge this branch: | bzr merge lp:~cody-somerville/launchpad/ppa-deletion | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Julian Edwards (community) | Approve | ||
Review via email: mp+22176@code.launchpad.net |
Commit message
Add support to publisher for deleting PPAs; and update generate_
Description of the change
Add a deleteArchive method to the publisher which will be called by the publishdistro script when an archive (for now just PPAs) has a status of ArchiveStatus.
Julian Edwards (julian-edwards) wrote : | # |
Cody A.W. Somerville (cody-somerville) wrote : | # |
On Thu, Mar 25, 2010 at 5:48 PM, Julian Edwards <
<email address hidden>> wrote:
> >
> > === modified file 'lib/lp/
> > --- lib/lp/
> +0000
> > +++ lib/lp/
> +0000
> > @@ -26,7 +26,7 @@
> > from canonical.
> > from canonical.
> > from lp.soyuz.
> > - ArchivePurpose, IArchiveSet)
> > + ArchivePurpose, ArchiveStatus, IArchiveSet)
> > from lp.soyuz.
> > BinaryPackageFo
> > from lp.registry.
> > @@ -95,6 +95,31 @@
> > foo_path = "%s/main/
> > self.assertEqua
> >
> > + def testDeletingPPA
> > + """Test deleting a PPA"""
> > + ubuntu_team = getUtility(
> > + test_archive = getUtility(
> > + distribution=
> > + purpose=
> > +
> > + publisher = getPublisher(
> > +
> > + pub_source = self.getPubSource(
> > + sourcename="foo", filename=
> > + filecontent='I am a file.',
> > + status=
> archive=
> > +
> > + publisher.
> > + self.layer.
> > + publisher.
> > + publisher.
> > + pub_source.sync()
>
> You don't need to do these last 5 lines, nor the getPubSource.
>
> Doing more than necessary makes for a fragile test if other parts of the
> system change. A better test would be to throw some random files into the
> root directory of the archive and then assert that they get deleted when
> you call deleteArchive, along with the status change.
>
You bring up a good point. However, I think what we need to do to
successfully test our ability to delete an archive and not just our ability
to delete a directory with some files in it is to attempt deleting a simple
but typical archive. If there is higher level way of achieving the same
result instead of calling those publisher methods manually then I think that
would be a good compromise. What do you think?
> > +
> > + self.assertTrue
> > + publisher.
> > + self.assertTrue(not
> os.path.
>
> How about assertFalse?
>
ACK.
<snip>
> === modified file 'lib/lp/
> > --- lib/lp/
> > +++ lib/lp/
> > @@ -16,7 +16,7 @@
> > from canonical.
> > clear_current_
> > from lp.soyuz.
>...
Julian Edwards (julian-edwards) wrote : | # |
On Friday 26 March 2010 01:15:27 Cody A.W. Somerville wrote:
> > Doing more than necessary makes for a fragile test if other parts of the
> > system change. A better test would be to throw some random files into
> > the root directory of the archive and then assert that they get deleted
> > when you call deleteArchive, along with the status change.
>
> You bring up a good point. However, I think what we need to do to
> successfully test our ability to delete an archive and not just our ability
> to delete a directory with some files in it is to attempt deleting a simple
> but typical archive. If there is higher level way of achieving the same
> result instead of calling those publisher methods manually then I think
> that would be a good compromise. What do you think?
In the nicest possible way, I think it's crazy.
The code you're testing doesn't care what you're deleting, it just does a
shutil.rmtree(). It's totally pointless putting any kind of data structure in
there because you're not using it; it just wastes test suite time. If we were
testing deleting of only certain parts of the archive, then of course it would
be correct to set up a real archive. But we're not.
Seriously, just poke some files in the archive root and test that they get
removed.
> > This code has got a subtle bug - it will only process archives marked
> > with publishing enabled because of the filter just above the context of
> > the diff.
>
> This was actually an intentional decision. Deleting an archive is a
> function of the publisher. If the publisher is disabled for an archive, I
> don't think we should process requests to delete the archive just like we
> wouldn't process pending publications or requests to copy a package into
> the archive.
>
> You're welcome to disagree on me this. I suspect that you might reply with
> that the flag's actual intention is to 'disable publishing for the
> archive', not 'disable the publisher for that archive'. However, I think
Correct :)
> even if this is the case then we should consider re-purposing it to
> include this slightly broader scope of functionality. If you're interested
> in my rationale or some potential use cases for this, I'd be happy to
> discuss it further with you on IRC.
>
> However, looking at your UI branch, it seems that you disable the PPA as
> well as set the status to ArchiveStatus.
> 'wrench' in this plan. See below for further thoughts related to this.
Think about this from the user's point of view (and the user's PoV should
drive everything). They're clicking a button that says to delete the PPA.
This is an action that has a certain amount of finality about it! Having to
then go and discover that it didn't really delete because you forgot to tick a
"publish" checkbox on the +edit page is rather weird, don't you think?
> > What do you think to adding IArchiveSet.
> > of the publisher script we can grab a list of archives that are in the
> > DELETING state and simply call publisher.
> > on IArchiveSet would require a simple new test.
>
> Could you elaborate on what exactly y...
Julian Edwards (julian-edwards) wrote : | # |
Oh one more thing:
lib/lp/
This needs changing so it ignores publish=False and status=DELETED otherwise I expect it'll blow up when the directory gets removed.
Cody A.W. Somerville (cody-somerville) wrote : | # |
> On Friday 26 March 2010 01:15:27 Cody A.W. Somerville wrote:
> > > Doing more than necessary makes for a fragile test if other parts of the
> > > system change. A better test would be to throw some random files into
> > > the root directory of the archive and then assert that they get deleted
> > > when you call deleteArchive, along with the status change.
> >
> > You bring up a good point. However, I think what we need to do to
> > successfully test our ability to delete an archive and not just our ability
> > to delete a directory with some files in it is to attempt deleting a simple
> > but typical archive. If there is higher level way of achieving the same
> > result instead of calling those publisher methods manually then I think
> > that would be a good compromise. What do you think?
>
> In the nicest possible way, I think it's crazy.
>
> The code you're testing doesn't care what you're deleting, it just does a
> shutil.rmtree(). It's totally pointless putting any kind of data structure in
> there because you're not using it; it just wastes test suite time. If we were
> testing deleting of only certain parts of the archive, then of course it would
> be correct to set up a real archive. But we're not.
>
> Seriously, just poke some files in the archive root and test that they get
> removed.
Okay.
> > > This code has got a subtle bug - it will only process archives marked
> > > with publishing enabled because of the filter just above the context of
> > > the diff.
> >
> > This was actually an intentional decision. Deleting an archive is a
> > function of the publisher. If the publisher is disabled for an archive, I
> > don't think we should process requests to delete the archive just like we
> > wouldn't process pending publications or requests to copy a package into
> > the archive.
> >
> > You're welcome to disagree on me this. I suspect that you might reply with
> > that the flag's actual intention is to 'disable publishing for the
> > archive', not 'disable the publisher for that archive'. However, I think
>
> Correct :)
>
> > even if this is the case then we should consider re-purposing it to
> > include this slightly broader scope of functionality. If you're interested
> > in my rationale or some potential use cases for this, I'd be happy to
> > discuss it further with you on IRC.
> >
> > However, looking at your UI branch, it seems that you disable the PPA as
> > well as set the status to ArchiveStatus.
> > 'wrench' in this plan. See below for further thoughts related to this.
>
> Think about this from the user's point of view (and the user's PoV should
> drive everything). They're clicking a button that says to delete the PPA.
> This is an action that has a certain amount of finality about it! Having to
> then go and discover that it didn't really delete because you forgot to tick a
> "publish" checkbox on the +edit page is rather weird, don't you think?
I don't feel strongly about this. However, the publish bool is not exposed to users (or on the form at all for anyone IIRC) so they'd only run into this if an admin intentionally disabled publishing for an archive. This I wo...
Cody A.W. Somerville (cody-somerville) wrote : | # |
> Oh one more thing:
>
> lib/lp/
>
> This needs changing so it ignores publish=False and status=DELETED otherwise I
> expect it'll blow up when the directory gets removed.
Why ignore archives that have publish=False?
Also, should I delete all subscriptions when we delete the archive or maybe update this script to do it? Or do you just want me to update it to ignore writing .htaccess file for deleted archives?
Julian Edwards (julian-edwards) wrote : | # |
Cody, thanks for all these changes it's looking great! There's just a couple of minor things to fix:
In lib/lp/
101 + open(os.path.join(
102 + publisher.
Please only tab in 4 chars, not 8.
In lib/lp/
132 + archives = [archive for archive in archives if archive.publish
133 + or archive.status == ArchiveStatus.
Our coding guidelines say to have a newline after the opening "["
So format it like this, which I think is more readable:
archives = [
archive for archive in archives
if archive.publish or archive.status == ArchiveStatus.
There's also a bug as I discussed on IRC:
158 + try_and_
159 + options.careful or options.
Should not have the extra args for deleteArchive().
Finally:
160 + else:
161 + # Other types of archives do not currently support deletion.
162 + pass
Please log a warning here.
After these changes it's good to land, thanks for the great job!
J
Julian Edwards (julian-edwards) wrote : | # |
On Thursday 15 April 2010 06:20:42 you wrote:
> > Oh one more thing:
> >
> > lib/lp/
> >
> > This needs changing so it ignores publish=False and status=DELETED
> > otherwise I expect it'll blow up when the directory gets removed.
>
> Why ignore archives that have publish=False?
Right, it should ignore archives that have disabled=True. Can you make that
change please :)
> Also, should I delete all subscriptions when we delete the archive or maybe
> update this script to do it? Or do you just want me to update it to ignore
> writing .htaccess file for deleted archives?
It just needs to ignore writing .htaccess, because the archiveroot won't be
there any more.
We can remove the subscriptions etc in a later branch when we do more of the
cleaning up.
Cheers.
Julian Edwards (julian-edwards) wrote : | # |
Cody, one last thing, the code you added in generate_
See lib/lp/
Preview Diff
1 | === modified file 'lib/lp/archivepublisher/publishing.py' |
2 | --- lib/lp/archivepublisher/publishing.py 2010-02-09 00:17:40 +0000 |
3 | +++ lib/lp/archivepublisher/publishing.py 2010-04-19 15:07:28 +0000 |
4 | @@ -12,6 +12,7 @@ |
5 | import hashlib |
6 | import logging |
7 | import os |
8 | +import shutil |
9 | |
10 | from datetime import datetime |
11 | |
12 | @@ -29,13 +30,15 @@ |
13 | from canonical.database.sqlbase import sqlvalues |
14 | from lp.registry.interfaces.pocket import ( |
15 | PackagePublishingPocket, pocketsuffix) |
16 | -from lp.soyuz.interfaces.archive import ArchivePurpose |
17 | +from lp.soyuz.interfaces.archive import ArchivePurpose, ArchiveStatus |
18 | from lp.soyuz.interfaces.binarypackagerelease import ( |
19 | BinaryPackageFormat) |
20 | from lp.soyuz.interfaces.component import IComponentSet |
21 | from lp.soyuz.interfaces.publishing import PackagePublishingStatus |
22 | |
23 | from canonical.librarian.client import LibrarianClient |
24 | +from canonical.launchpad.webapp.errorlog import ( |
25 | + ErrorReportingUtility, ScriptRequest) |
26 | |
27 | suffixpocket = dict((v, k) for (k, v) in pocketsuffix.items()) |
28 | |
29 | @@ -596,3 +599,39 @@ |
30 | in_file.close() |
31 | |
32 | out_file.write(" %s % 16d %s\n" % (checksum, length, file_name)) |
33 | + |
34 | + def deleteArchive(self): |
35 | + """Delete the archive. |
36 | + |
37 | + Physically remove the entire archive from disk and set the archive's |
38 | + status to DELETED. |
39 | + |
40 | + Any errors encountered while removing the archive from disk will |
41 | + be caught and an OOPS report generated. |
42 | + """ |
43 | + |
44 | + self.log.info( |
45 | + "Attempting to delete archive '%s/%s' at '%s'." % ( |
46 | + self.archive.owner.name, self.archive.name, |
47 | + self._config.archiveroot)) |
48 | + |
49 | + # Attempt to rmdir if the path to the root of the archive exists. |
50 | + if os.path.exists(self._config.archiveroot): |
51 | + try: |
52 | + shutil.rmtree(self._config.archiveroot) |
53 | + except: |
54 | + message = 'Exception while deleting archive root %s' % ( |
55 | + self._config.archiveroot) |
56 | + request = ScriptRequest([('error-explanation', message)]) |
57 | + ErrorReportingUtility().raising(sys.exc_info(), request) |
58 | + self.log.error('%s (%s)' % (message, request.oopsid)) |
59 | + else: |
60 | + self.log.warning( |
61 | + "Root directory '%s' for archive '%s/%s' does not exist." % ( |
62 | + self._config.archiveroot, self.archive.owner.name, |
63 | + self.archive.name)) |
64 | + |
65 | + # Set archive's status to DELETED. |
66 | + self.archive.status = ArchiveStatus.DELETED |
67 | + # Disable publishing for this archive. |
68 | + self.archive.publish = False |
69 | |
70 | === modified file 'lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py' |
71 | --- lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2009-10-17 14:06:03 +0000 |
72 | +++ lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2010-04-19 15:07:28 +0000 |
73 | @@ -17,7 +17,7 @@ |
74 | from canonical.launchpad.webapp import canonical_url |
75 | |
76 | from lp.archivepublisher.config import getPubConfig |
77 | -from lp.soyuz.interfaces.archive import IArchiveSet |
78 | +from lp.soyuz.interfaces.archive import IArchiveSet, ArchiveStatus |
79 | from lp.soyuz.interfaces.archiveauthtoken import ( |
80 | IArchiveAuthTokenSet) |
81 | from lp.soyuz.interfaces.archivesubscriber import ( |
82 | @@ -241,6 +241,13 @@ |
83 | ppa.name, |
84 | ppa.owner.displayname) |
85 | continue |
86 | + elif ppa.status == ArchiveStatus.DELETED or ppa.enabled is False: |
87 | + self.logger.info( |
88 | + "Skipping htacess updates for deleted or disabled PPA " |
89 | + " '%s' owned by %s.", |
90 | + ppa.name, |
91 | + ppa.owner.displayname) |
92 | + continue |
93 | |
94 | self.ensureHtaccess(ppa) |
95 | temp_htpasswd = self.generateHtpasswd(ppa, valid_tokens) |
96 | |
97 | === modified file 'lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py' |
98 | --- lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py 2010-03-09 07:29:18 +0000 |
99 | +++ lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py 2010-04-19 15:07:28 +0000 |
100 | @@ -24,6 +24,7 @@ |
101 | from lp.archivepublisher.config import getPubConfig |
102 | from lp.archivepublisher.scripts.generate_ppa_htaccess import ( |
103 | HtaccessTokenGenerator) |
104 | +from lp.soyuz.interfaces.archive import ArchiveStatus |
105 | from lp.soyuz.interfaces.archivesubscriber import ( |
106 | ArchiveSubscriberStatus) |
107 | from lp.testing import TestCaseWithFactory |
108 | @@ -476,6 +477,47 @@ |
109 | self.assertFalse(os.path.isfile(htaccess)) |
110 | self.assertFalse(os.path.isfile(htpasswd)) |
111 | |
112 | + def testSkippingOfDisabledPPAs(self): |
113 | + """Test that the htaccess for disabled PPAs are not touched.""" |
114 | + subs, tokens = self.setupDummyTokens() |
115 | + htaccess, htpasswd = self.ensureNoFiles() |
116 | + |
117 | + # Setup subscription so that htaccess/htpasswd is pending generation. |
118 | + now = datetime.now(pytz.UTC) |
119 | + subs[0].date_expires = now + timedelta(minutes=3) |
120 | + self.assertEqual(subs[0].status, ArchiveSubscriberStatus.CURRENT) |
121 | + |
122 | + # Set the PPA as disabled. |
123 | + self.ppa.disable() |
124 | + self.assertFalse(self.ppa.enabled) |
125 | + |
126 | + script = self.getScript() |
127 | + script.main() |
128 | + |
129 | + # The htaccess and htpasswd files should not be generated. |
130 | + self.assertFalse(os.path.isfile(htaccess)) |
131 | + self.assertFalse(os.path.isfile(htpasswd)) |
132 | + |
133 | + def testSkippingOfDeletedPPAs(self): |
134 | + """Test that the htaccess for deleted PPAs are not touched.""" |
135 | + subs, tokens = self.setupDummyTokens() |
136 | + htaccess, htpasswd = self.ensureNoFiles() |
137 | + |
138 | + # Setup subscription so that htaccess/htpasswd is pending generation. |
139 | + now = datetime.now(pytz.UTC) |
140 | + subs[0].date_expires = now + timedelta(minutes=3) |
141 | + self.assertEqual(subs[0].status, ArchiveSubscriberStatus.CURRENT) |
142 | + |
143 | + # Set the PPA as deleted. |
144 | + self.ppa.status = ArchiveStatus.DELETED |
145 | + |
146 | + script = self.getScript() |
147 | + script.main() |
148 | + |
149 | + # The htaccess and htpasswd files should not be generated. |
150 | + self.assertFalse(os.path.isfile(htaccess)) |
151 | + self.assertFalse(os.path.isfile(htpasswd)) |
152 | + |
153 | def testSendingCancellationEmail(self): |
154 | """Test that when a token is deactivated, its user gets an email. |
155 | |
156 | |
157 | === modified file 'lib/lp/archivepublisher/tests/test_publisher.py' |
158 | --- lib/lp/archivepublisher/tests/test_publisher.py 2010-02-10 00:25:55 +0000 |
159 | +++ lib/lp/archivepublisher/tests/test_publisher.py 2010-04-19 15:07:28 +0000 |
160 | @@ -26,7 +26,7 @@ |
161 | from canonical.database.constants import UTC_NOW |
162 | from canonical.launchpad.ftests.keys_for_tests import gpgkeysdir |
163 | from lp.soyuz.interfaces.archive import ( |
164 | - ArchivePurpose, IArchiveSet) |
165 | + ArchivePurpose, ArchiveStatus, IArchiveSet) |
166 | from lp.soyuz.interfaces.binarypackagerelease import ( |
167 | BinaryPackageFormat) |
168 | from lp.registry.interfaces.distribution import IDistributionSet |
169 | @@ -95,6 +95,24 @@ |
170 | foo_path = "%s/main/f/foo/foo_666.dsc" % self.pool_dir |
171 | self.assertEqual(open(foo_path).read().strip(), 'Hello world') |
172 | |
173 | + def testDeletingPPA(self): |
174 | + """Test deleting a PPA""" |
175 | + ubuntu_team = getUtility(IPersonSet).getByName('ubuntu-team') |
176 | + test_archive = getUtility(IArchiveSet).new( |
177 | + distribution=self.ubuntutest, owner=ubuntu_team, |
178 | + purpose=ArchivePurpose.PPA) |
179 | + publisher = getPublisher(test_archive, None, self.logger) |
180 | + |
181 | + self.assertTrue(os.path.exists(publisher._config.archiveroot)) |
182 | + |
183 | + # Create a file inside archiveroot to ensure we're recursive. |
184 | + open(os.path.join( |
185 | + publisher._config.archiveroot, 'test_file'), 'w').close() |
186 | + |
187 | + publisher.deleteArchive() |
188 | + self.assertFalse(os.path.exists(publisher._config.archiveroot)) |
189 | + self.assertEqual(test_archive.status, ArchiveStatus.DELETED) |
190 | + |
191 | def testPublishPartner(self): |
192 | """Test that a partner package is published to the right place.""" |
193 | archive = self.ubuntutest.getArchiveByComponent('partner') |
194 | |
195 | === modified file 'lib/lp/soyuz/scripts/publishdistro.py' |
196 | --- lib/lp/soyuz/scripts/publishdistro.py 2010-02-24 11:53:22 +0000 |
197 | +++ lib/lp/soyuz/scripts/publishdistro.py 2010-04-19 15:07:28 +0000 |
198 | @@ -16,7 +16,7 @@ |
199 | from canonical.database.sqlbase import ( |
200 | clear_current_connection_cache, flush_database_updates) |
201 | from lp.soyuz.interfaces.archive import ( |
202 | - ArchivePurpose, IArchiveSet, MAIN_ARCHIVE_PURPOSES) |
203 | + ArchivePurpose, ArchiveStatus, IArchiveSet, MAIN_ARCHIVE_PURPOSES) |
204 | from lp.registry.interfaces.distribution import IDistributionSet |
205 | from canonical.launchpad.scripts import logger, logger_options |
206 | from lp.services.scripts.base import LaunchpadScriptFailure |
207 | @@ -178,8 +178,11 @@ |
208 | else: |
209 | archives = [distribution.main_archive] |
210 | |
211 | - # Consider only archives that have their "to be published" flag turned on. |
212 | - archives = [archive for archive in archives if archive.publish] |
213 | + # Consider only archives that have their "to be published" flag turned on |
214 | + # or are pending deletion. |
215 | + archives = [ |
216 | + archive for archive in archives |
217 | + if archive.publish or archive.status == ArchiveStatus.DELETING] |
218 | |
219 | for archive in archives: |
220 | if archive.purpose in MAIN_ARCHIVE_PURPOSES: |
221 | @@ -191,24 +194,34 @@ |
222 | else: |
223 | log.info("Processing %s" % archive.archive_url) |
224 | publisher = getPublisher(archive, allowed_suites, log) |
225 | - |
226 | - try_and_commit("publishing", publisher.A_publish, |
227 | - options.careful or options.careful_publishing) |
228 | - # Flag dirty pockets for any outstanding deletions. |
229 | - publisher.A2_markPocketsWithDeletionsDirty() |
230 | - try_and_commit("dominating", publisher.B_dominate, |
231 | - options.careful or options.careful_domination) |
232 | - |
233 | - # The primary and copy archives use apt-ftparchive to generate the |
234 | - # indexes, everything else uses the newer internal LP code. |
235 | - if archive.purpose in (ArchivePurpose.PRIMARY, ArchivePurpose.COPY): |
236 | - try_and_commit("doing apt-ftparchive", publisher.C_doFTPArchive, |
237 | - options.careful or options.careful_apt) |
238 | + |
239 | + # Do we need to delete the archive or publish it? |
240 | + if archive.status == ArchiveStatus.DELETING: |
241 | + if archive.purpose == ArchivePurpose.PPA: |
242 | + try_and_commit("deleting archive", publisher.deleteArchive) |
243 | + else: |
244 | + # Other types of archives do not currently support deletion. |
245 | + log.warning( |
246 | + "Deletion of %s skipped: operation not supported on %s" |
247 | + % archive.displayname) |
248 | else: |
249 | - try_and_commit("building indexes", publisher.C_writeIndexes, |
250 | + try_and_commit("publishing", publisher.A_publish, |
251 | + options.careful or options.careful_publishing) |
252 | + # Flag dirty pockets for any outstanding deletions. |
253 | + publisher.A2_markPocketsWithDeletionsDirty() |
254 | + try_and_commit("dominating", publisher.B_dominate, |
255 | + options.careful or options.careful_domination) |
256 | + |
257 | + # The primary and copy archives use apt-ftparchive to generate the |
258 | + # indexes, everything else uses the newer internal LP code. |
259 | + if archive.purpose in (ArchivePurpose.PRIMARY, ArchivePurpose.COPY): |
260 | + try_and_commit("doing apt-ftparchive", publisher.C_doFTPArchive, |
261 | + options.careful or options.careful_apt) |
262 | + else: |
263 | + try_and_commit("building indexes", publisher.C_writeIndexes, |
264 | + options.careful or options.careful_apt) |
265 | + |
266 | + try_and_commit("doing release files", publisher.D_writeReleaseFiles, |
267 | options.careful or options.careful_apt) |
268 | |
269 | - try_and_commit("doing release files", publisher.D_writeReleaseFiles, |
270 | - options.careful or options.careful_apt) |
271 | - |
272 | log.debug("Ciao") |
> === modified file 'lib/lp/ archivepublishe r/publishing. py' archivepublishe r/publishing. py 2010-02-09 00:17:40 +0000 archivepublishe r/publishing. py 2010-03-25 20:23:14 +0000 COMPONENT_ ORDER database. sqlbase import sqlvalues interfaces. pocket import ( ngPocket, pocketsuffix) interfaces. archive import ArchivePurpose interfaces. archive import ArchivePurpose, ArchiveStatus interfaces. binarypackagere lease import ( rmat) interfaces. component import IComponentSet interfaces. publishing import PackagePublishi ngStatus librarian. client import LibrarianClient launchpad. webapp. errorlog import ( tility, ScriptRequest) items() ) self): owner.name, self.archive.name, archiveroot) ) exists( self._config. archiveroot) : rmtree( self._config. archiveroot) archiveroot) [('error- explanation' , message)]) tility( ).raising( sys.exc_ info(), request) archiveroot, self.archive. owner.name, DELETED
> --- lib/lp/
> +++ lib/lp/
> @@ -12,9 +12,11 @@
> import hashlib
> import logging
> import os
> +import shutil
>
> from datetime import datetime
>
> +from storm.store import Store
> from zope.component import getUtility
>
> from lp.archivepublisher import HARDCODED_
> @@ -29,13 +31,15 @@
> from canonical.
> from lp.registry.
> PackagePublishi
> -from lp.soyuz.
> +from lp.soyuz.
> from lp.soyuz.
> BinaryPackageFo
> from lp.soyuz.
> from lp.soyuz.
>
> from canonical.
> +from canonical.
> + ErrorReportingU
>
> suffixpocket = dict((v, k) for (k, v) in pocketsuffix.
>
> @@ -596,3 +600,37 @@
> in_file.close()
>
> out_file.write(" %s % 16d %s\n" % (checksum, length, file_name))
> +
> + def deleteArchive(
> + """Delete the archive.
> +
> + Physically remove the entire archive from disk and set the
archive's
> + status to DELETED.
> +
> + Any errors encountered while removing the archive from disk will
> + be caught and an OOPS report generated.
> + """
> +
> + self.log.info(
> + "Attempting to delete archive '%s/%s' at '%s'." % (
> + self.archive.
> + self._config.
> +
> + # Attempt to rmdir if the path to the root of the archive exists.
> + if os.path.
> + try:
> + shutil.
> + except:
> + message = 'Exception while deleting archive root %s' % (
> + self._config.
> + request = ScriptRequest(
> + ErrorReportingU
> + self.log.error('%s (%s)' % (message, request.oopsid))
> + else:
> + self.log.warning(
> + "Root directory '%s' for archive '%s/%s' does not exist." %
(
> + self._config.
> + self.archive.name))
> +
> + # Set archive's status to DELETED.
> + self.archive.status = ArchiveStatus.
This is all good. I was trying to think of failure scenarios with it but I
think you have them all covered!
> archivepublishe r/tests/ test_publisher. py' archivepublishe r/tests/ test_publisher. py 2010-02-10 00:25:55 archivepublishe r/tests/ test_publisher. py 2010-03-25 20:23:14
> === modified file 'lib/lp/
> --- lib/lp/
+0000
> +++ lib/lp/
+0000
> @@ -26,7 +2...