Merge lp:~jelmer/launchpad/oops-on-pool-overwrite-error into lp:launchpad

Proposed by Jelmer Vernooij
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: 11191
Proposed branch: lp:~jelmer/launchpad/oops-on-pool-overwrite-error
Merge into: lp:launchpad
Prerequisite: lp:~jelmer/launchpad/robust-process-accepted
Diff against target: 126 lines (+39/-20)
3 files modified
lib/lp/archiveuploader/tests/test_uploadprocessor.py (+10/-0)
lib/lp/soyuz/model/publishing.py (+19/-20)
lib/lp/soyuz/tests/test_publishing.py (+10/-0)
To merge this branch: bzr merge lp:~jelmer/launchpad/oops-on-pool-overwrite-error
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+23328@code.launchpad.net

Commit message

File an OOPS rather than just logging an error when encountering a file with a different checksum in process-accepted.

Description of the change

Previously process-accepted would just log errors when it encountered files with a different checksum. This patch makes it file OOPSes instead.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Thanks for the change and the clarification on IRC. Please do consider using our tools to help create better merge proposal messages.

In your test you have at least one comment that needs final punctuation.

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/archiveuploader/tests/test_uploadprocessor.py'
2--- lib/lp/archiveuploader/tests/test_uploadprocessor.py 2010-07-18 00:26:33 +0000
3+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py 2010-07-21 14:21:58 +0000
4@@ -1155,6 +1155,16 @@
5 expect_msg in raw_msg,
6 "Expected email with %s, got:\n%s" % (expect_msg, raw_msg))
7
8+
9+ # And an oops should be filed for the error.
10+ error_utility = ErrorReportingUtility()
11+ error_report = error_utility.getLastOopsReport()
12+ fp = StringIO()
13+ error_report.write(fp)
14+ error_text = fp.getvalue()
15+ self.assertTrue(
16+ "Unable to find mandatory field 'files' in the changes file" in error_text)
17+
18 # Housekeeping so the next test won't fail.
19 shutil.rmtree(upload_dir)
20
21
22=== modified file 'lib/lp/soyuz/model/publishing.py'
23--- lib/lp/soyuz/model/publishing.py 2010-07-21 09:41:48 +0000
24+++ lib/lp/soyuz/model/publishing.py 2010-07-21 14:21:58 +0000
25@@ -22,6 +22,7 @@
26 import os
27 import pytz
28 import re
29+import sys
30
31 from zope.component import getUtility
32 from zope.interface import implements
33@@ -40,6 +41,8 @@
34 from canonical.launchpad.interfaces.lpstorm import IMasterStore
35 from canonical.launchpad.webapp.interfaces import (
36 IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
37+from canonical.launchpad.webapp.errorlog import (
38+ ErrorReportingUtility, ScriptRequest)
39 from canonical.launchpad.webapp.interfaces import NotFoundError
40 from lp.buildmaster.interfaces.buildbase import BuildStatus
41 from lp.buildmaster.model.buildfarmjob import BuildFarmJob
42@@ -64,8 +67,7 @@
43 active_publishing_status, IBinaryPackageFilePublishing,
44 IBinaryPackagePublishingHistory, IPublishingSet,
45 ISourcePackageFilePublishing, ISourcePackagePublishingHistory,
46- PackagePublishingPriority, PackagePublishingStatus,
47- PoolFileOverwriteError)
48+ PackagePublishingPriority, PackagePublishingStatus, PoolFileOverwriteError)
49 from lp.soyuz.interfaces.queue import PackageUploadStatus
50 from lp.soyuz.pas import determineArchitecturesToBuild
51 from lp.soyuz.scripts.changeoverride import ArchiveOverriderError
52@@ -97,22 +99,14 @@
53 sha1 = filealias.content.sha1
54 path = diskpool.pathFor(component, source, filename)
55
56- try:
57- action = diskpool.addFile(
58- component, source, filename, sha1, filealias)
59- if action == diskpool.results.FILE_ADDED:
60- log.debug("Added %s from library" % path)
61- elif action == diskpool.results.SYMLINK_ADDED:
62- log.debug("%s created as a symlink." % path)
63- elif action == diskpool.results.NONE:
64- log.debug(
65- "%s is already in pool with the same content." % path)
66- except PoolFileOverwriteError, info:
67- log.error("PoolFileOverwriteError: %s. Skipping. This indicates "
68- "some bad data, and Team Soyuz should be informed. "
69- "However, publishing of other packages is not affected."
70- % info)
71- raise info
72+ action = diskpool.addFile(component, source, filename, sha1, filealias)
73+ if action == diskpool.results.FILE_ADDED:
74+ log.debug("Added %s from library" % path)
75+ elif action == diskpool.results.SYMLINK_ADDED:
76+ log.debug("%s created as a symlink." % path)
77+ elif action == diskpool.results.NONE:
78+ log.debug(
79+ "%s is already in pool with the same content." % path)
80
81 @property
82 def archive_url(self):
83@@ -262,8 +256,13 @@
84 try:
85 for pub_file in self.files:
86 pub_file.publish(diskpool, log)
87- except PoolFileOverwriteError:
88- pass
89+ except PoolFileOverwriteError, e:
90+ message = "PoolFileOverwriteError: %s, skipping." % e
91+ properties = [('error-explanation', message)]
92+ request = ScriptRequest(properties)
93+ error_utility = ErrorReportingUtility()
94+ error_utility.raising(sys.exc_info(), request)
95+ log.error('%s (%s)' % (message, request.oopsid))
96 else:
97 self.setPublished()
98
99
100=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
101--- lib/lp/soyuz/tests/test_publishing.py 2010-07-21 07:45:50 +0000
102+++ lib/lp/soyuz/tests/test_publishing.py 2010-07-21 14:21:58 +0000
103@@ -18,6 +18,7 @@
104 from canonical.config import config
105 from canonical.database.constants import UTC_NOW
106 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
107+from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
108 from canonical.launchpad.webapp.interfaces import NotFoundError
109 from canonical.testing import (
110 DatabaseFunctionalLayer, LaunchpadZopelessLayer)
111@@ -598,6 +599,15 @@
112
113 pub_source = self.getPubSource(filecontent="Something")
114 pub_source.publish(self.disk_pool, self.logger)
115+
116+ # And an oops should be filed for the error.
117+ error_utility = ErrorReportingUtility()
118+ error_report = error_utility.getLastOopsReport()
119+ fp = StringIO()
120+ error_report.write(fp)
121+ error_text = fp.getvalue()
122+ self.assertTrue("PoolFileOverwriteError" in error_text)
123+
124 self.layer.commit()
125 self.assertEqual(
126 pub_source.status, PackagePublishingStatus.PENDING)