Launchpad itself

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

  1. oops-on-pool-overwrite-error
  2. Merge into devel
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
=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py 2010-07-18 00:26:33 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py 2010-07-21 14:21:58 +0000
@@ -1155,6 +1155,16 @@
1155 expect_msg in raw_msg,1155 expect_msg in raw_msg,
1156 "Expected email with %s, got:\n%s" % (expect_msg, raw_msg))1156 "Expected email with %s, got:\n%s" % (expect_msg, raw_msg))
11571157
1158
1159 # And an oops should be filed for the error.
1160 error_utility = ErrorReportingUtility()
1161 error_report = error_utility.getLastOopsReport()
1162 fp = StringIO()
1163 error_report.write(fp)
1164 error_text = fp.getvalue()
1165 self.assertTrue(
1166 "Unable to find mandatory field 'files' in the changes file" in error_text)
1167
1158 # Housekeeping so the next test won't fail.1168 # Housekeeping so the next test won't fail.
1159 shutil.rmtree(upload_dir)1169 shutil.rmtree(upload_dir)
11601170
11611171
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2010-07-21 09:41:48 +0000
+++ lib/lp/soyuz/model/publishing.py 2010-07-21 14:21:58 +0000
@@ -22,6 +22,7 @@
22import os22import os
23import pytz23import pytz
24import re24import re
25import sys
2526
26from zope.component import getUtility27from zope.component import getUtility
27from zope.interface import implements28from zope.interface import implements
@@ -40,6 +41,8 @@
40from canonical.launchpad.interfaces.lpstorm import IMasterStore41from canonical.launchpad.interfaces.lpstorm import IMasterStore
41from canonical.launchpad.webapp.interfaces import (42from canonical.launchpad.webapp.interfaces import (
42 IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)43 IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
44from canonical.launchpad.webapp.errorlog import (
45 ErrorReportingUtility, ScriptRequest)
43from canonical.launchpad.webapp.interfaces import NotFoundError46from canonical.launchpad.webapp.interfaces import NotFoundError
44from lp.buildmaster.interfaces.buildbase import BuildStatus47from lp.buildmaster.interfaces.buildbase import BuildStatus
45from lp.buildmaster.model.buildfarmjob import BuildFarmJob48from lp.buildmaster.model.buildfarmjob import BuildFarmJob
@@ -64,8 +67,7 @@
64 active_publishing_status, IBinaryPackageFilePublishing,67 active_publishing_status, IBinaryPackageFilePublishing,
65 IBinaryPackagePublishingHistory, IPublishingSet,68 IBinaryPackagePublishingHistory, IPublishingSet,
66 ISourcePackageFilePublishing, ISourcePackagePublishingHistory,69 ISourcePackageFilePublishing, ISourcePackagePublishingHistory,
67 PackagePublishingPriority, PackagePublishingStatus,70 PackagePublishingPriority, PackagePublishingStatus, PoolFileOverwriteError)
68 PoolFileOverwriteError)
69from lp.soyuz.interfaces.queue import PackageUploadStatus71from lp.soyuz.interfaces.queue import PackageUploadStatus
70from lp.soyuz.pas import determineArchitecturesToBuild72from lp.soyuz.pas import determineArchitecturesToBuild
71from lp.soyuz.scripts.changeoverride import ArchiveOverriderError73from lp.soyuz.scripts.changeoverride import ArchiveOverriderError
@@ -97,22 +99,14 @@
97 sha1 = filealias.content.sha199 sha1 = filealias.content.sha1
98 path = diskpool.pathFor(component, source, filename)100 path = diskpool.pathFor(component, source, filename)
99101
100 try:102 action = diskpool.addFile(component, source, filename, sha1, filealias)
101 action = diskpool.addFile(103 if action == diskpool.results.FILE_ADDED:
102 component, source, filename, sha1, filealias)104 log.debug("Added %s from library" % path)
103 if action == diskpool.results.FILE_ADDED:105 elif action == diskpool.results.SYMLINK_ADDED:
104 log.debug("Added %s from library" % path)106 log.debug("%s created as a symlink." % path)
105 elif action == diskpool.results.SYMLINK_ADDED:107 elif action == diskpool.results.NONE:
106 log.debug("%s created as a symlink." % path)108 log.debug(
107 elif action == diskpool.results.NONE:109 "%s is already in pool with the same content." % path)
108 log.debug(
109 "%s is already in pool with the same content." % path)
110 except PoolFileOverwriteError, info:
111 log.error("PoolFileOverwriteError: %s. Skipping. This indicates "
112 "some bad data, and Team Soyuz should be informed. "
113 "However, publishing of other packages is not affected."
114 % info)
115 raise info
116110
117 @property111 @property
118 def archive_url(self):112 def archive_url(self):
@@ -262,8 +256,13 @@
262 try:256 try:
263 for pub_file in self.files:257 for pub_file in self.files:
264 pub_file.publish(diskpool, log)258 pub_file.publish(diskpool, log)
265 except PoolFileOverwriteError:259 except PoolFileOverwriteError, e:
266 pass260 message = "PoolFileOverwriteError: %s, skipping." % e
261 properties = [('error-explanation', message)]
262 request = ScriptRequest(properties)
263 error_utility = ErrorReportingUtility()
264 error_utility.raising(sys.exc_info(), request)
265 log.error('%s (%s)' % (message, request.oopsid))
267 else:266 else:
268 self.setPublished()267 self.setPublished()
269268
270269
=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py 2010-07-21 07:45:50 +0000
+++ lib/lp/soyuz/tests/test_publishing.py 2010-07-21 14:21:58 +0000
@@ -18,6 +18,7 @@
18from canonical.config import config18from canonical.config import config
19from canonical.database.constants import UTC_NOW19from canonical.database.constants import UTC_NOW
20from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet20from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
21from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
21from canonical.launchpad.webapp.interfaces import NotFoundError22from canonical.launchpad.webapp.interfaces import NotFoundError
22from canonical.testing import (23from canonical.testing import (
23 DatabaseFunctionalLayer, LaunchpadZopelessLayer)24 DatabaseFunctionalLayer, LaunchpadZopelessLayer)
@@ -598,6 +599,15 @@
598599
599 pub_source = self.getPubSource(filecontent="Something")600 pub_source = self.getPubSource(filecontent="Something")
600 pub_source.publish(self.disk_pool, self.logger)601 pub_source.publish(self.disk_pool, self.logger)
602
603 # And an oops should be filed for the error.
604 error_utility = ErrorReportingUtility()
605 error_report = error_utility.getLastOopsReport()
606 fp = StringIO()
607 error_report.write(fp)
608 error_text = fp.getvalue()
609 self.assertTrue("PoolFileOverwriteError" in error_text)
610
601 self.layer.commit()611 self.layer.commit()
602 self.assertEqual(612 self.assertEqual(
603 pub_source.status, PackagePublishingStatus.PENDING)613 pub_source.status, PackagePublishingStatus.PENDING)