Merge lp:~jelmer/launchpad/128259-async-1 into lp:launchpad

Proposed by Jelmer Vernooij
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: 11399
Proposed branch: lp:~jelmer/launchpad/128259-async-1
Merge into: lp:launchpad
Prerequisite: lp:~jelmer/launchpad/refactor-uploadprocessor
Diff against target: 559 lines (+230/-44)
4 files modified
lib/lp/archiveuploader/tests/test_uploadprocessor.py (+112/-7)
lib/lp/archiveuploader/uploadpolicy.py (+2/-1)
lib/lp/archiveuploader/uploadprocessor.py (+108/-33)
lib/lp/soyuz/scripts/soyuz_process_upload.py (+8/-3)
To merge this branch: bzr merge lp:~jelmer/launchpad/128259-async-1
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+32439@code.launchpad.net

Commit message

Add --builds option to process-upload.

Description of the change

This branch adds a --builds option to process-upload.

This new option makes process-upload the directory name in which the upload was found as <buildid>-<buildqueuerecordid>, and update the build status appropriately after processing the upload. This is the name format that is already used by the buildd manager.

This makes it possible for uploads of builds to no longer happen synchronously inside of the buildd manager, but rather for the buildd manager to move them into a separate queue where they can be processed by a process-upload independently without blocking the buildd manager.

To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) wrote :

It looks like that there are conflicts with this branch. Could you please resolve them?

Revision history for this message
Abel Deuring (adeuring) wrote :
Download full text (5.1 KiB)

Hi Jelmer,

a nice branch! I have just a few formal nitpicks, see below.

> === modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
> --- lib/lp/archiveuploader/tests/test_uploadprocessor.py 2010-08-06 13:52:46 +0000
> +++ lib/lp/archiveuploader/tests/test_uploadprocessor.py 2010-08-12 21:09:28 +0000

[...]

> + def testSuccess(self):
> + # Upload a source package
> + upload_dir = self.queueUpload("bar_1.0-1")
> + self.processUpload(self.uploadprocessor, upload_dir)
> + source_pub = self.publishPackage('bar', '1.0-1')
> + [build] = source_pub.createMissingBuilds()
> +
> + # Move the source from the accepted queue.
> + [queue_item] = self.breezy.getQueueItems(
> + status=PackageUploadStatus.ACCEPTED,
> + version="1.0-1", name="bar")
> + queue_item.setDone()
> +
> + build.jobStarted()
> + build.builder = self.factory.makeBuilder()
> +
> + # Upload and accept a binary for the primary archive source.
> + shutil.rmtree(upload_dir)
> + self.layer.txn.commit()
> + leaf_name = "%d-%d" % (build.id, 60)
> + upload_dir = self.queueUpload("bar_1.0-1_binary",
> + queue_entry=leaf_name)
> + self.options.context = 'buildd'
> + self.options.builds = True
> + self.uploadprocessor.processBuildUpload(self.incoming_folder, leaf_name)
> + self.layer.txn.commit()
> + self.assertEquals(BuildStatus.FULLYBUILT, build.status)
> + log_lines = build.upload_log.read().splitlines()
> + self.assertTrue(
> + 'INFO: Processing upload bar_1.0-1_i386.changes' in log_lines)
> + self.assertTrue(
> + 'INFO: Committing the transaction and any mails associated with'
> + 'this upload.')

As already mentioned on IRC, this should be something like

    assertTrue('INFO...' in log_lines)

(And I assume that a ' ' is missing after 'with'.)

> === modified file 'lib/lp/archiveuploader/uploadprocessor.py'
> --- lib/lp/archiveuploader/uploadprocessor.py 2010-08-12 17:54:00 +0000
> +++ lib/lp/archiveuploader/uploadprocessor.py 2010-08-12 21:09:28 +0000

[...]

> @@ -184,7 +257,8 @@
> for changes_file in changes_files:
> self.log.debug("Considering changefile %s" % changes_file)
> try:
> - result = self.processChangesFile(upload_path, changes_file)
> + result = self.processChangesFile(upload_path, changes_file,
> + self.log)

Our style guide says that we should put all parameters into the second
(and following) line if they don't fit completely into the first line.

[...]

> @@ -376,11 +451,11 @@
> except UploadPolicyError, e:
> upload.reject("UploadPolicyError escaped upload.process: "
> "%s " % e)
> - self.log.debug("UploadPolicyError escaped upload.process",
> + logger.debug("UploadPolicyError escaped upload.process",
> exc_info=True)

Again, please move all parameters to the second line.

> except FatalUploadError, e:
> ...

Read more...

review: Approve (code)
Revision history for this message
Abel Deuring (adeuring) wrote :

ah, one more detail: line 269 of the diff is

+ self.ztm.commit()

could you add a comment why the commit() call is necessary?

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-08-18 14:03:15 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py 2010-08-19 14:33:31 +0000
@@ -25,7 +25,9 @@
25from lp.app.errors import NotFoundError25from lp.app.errors import NotFoundError
26from lp.archiveuploader.uploadpolicy import (26from lp.archiveuploader.uploadpolicy import (
27 AbstractUploadPolicy, IArchiveUploadPolicy, findPolicyByName)27 AbstractUploadPolicy, IArchiveUploadPolicy, findPolicyByName)
28from lp.archiveuploader.uploadprocessor import UploadProcessor28from lp.archiveuploader.uploadprocessor import (UploadProcessor,
29 parse_build_upload_leaf_name)
30from lp.buildmaster.interfaces.buildbase import BuildStatus
29from canonical.config import config31from canonical.config import config
30from canonical.database.constants import UTC_NOW32from canonical.database.constants import UTC_NOW
31from lp.soyuz.model.archivepermission import ArchivePermission33from lp.soyuz.model.archivepermission import ArchivePermission
@@ -61,7 +63,10 @@
61 ISourcePackageNameSet)63 ISourcePackageNameSet)
62from lp.services.mail import stub64from lp.services.mail import stub
63from canonical.launchpad.testing.fakepackager import FakePackager65from canonical.launchpad.testing.fakepackager import FakePackager
64from lp.testing import TestCaseWithFactory66from lp.testing import (
67 TestCase,
68 TestCaseWithFactory,
69 )
65from lp.testing.mail_helpers import pop_notifications70from lp.testing.mail_helpers import pop_notifications
66from canonical.launchpad.webapp.errorlog import ErrorReportingUtility71from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
67from canonical.testing import LaunchpadZopelessLayer72from canonical.testing import LaunchpadZopelessLayer
@@ -125,6 +130,7 @@
125130
126 self.options = MockOptions()131 self.options = MockOptions()
127 self.options.base_fsroot = self.queue_folder132 self.options.base_fsroot = self.queue_folder
133 self.options.builds = True
128 self.options.leafname = None134 self.options.leafname = None
129 self.options.distro = "ubuntu"135 self.options.distro = "ubuntu"
130 self.options.distroseries = None136 self.options.distroseries = None
@@ -150,7 +156,7 @@
150 return policy156 return policy
151 return UploadProcessor(157 return UploadProcessor(
152 self.options.base_fsroot, self.options.dryrun,158 self.options.base_fsroot, self.options.dryrun,
153 self.options.nomails,159 self.options.nomails, self.options.builds,
154 self.options.keep, getPolicy, txn, self.log)160 self.options.keep, getPolicy, txn, self.log)
155161
156 def publishPackage(self, packagename, version, source=True,162 def publishPackage(self, packagename, version, source=True,
@@ -434,7 +440,7 @@
434 # Move it440 # Move it
435 self.options.base_fsroot = testdir441 self.options.base_fsroot = testdir
436 up = self.getUploadProcessor(None)442 up = self.getUploadProcessor(None)
437 up.moveUpload(upload, target_name)443 up.moveUpload(upload, target_name, self.log)
438444
439 # Check it moved445 # Check it moved
440 self.assertTrue(os.path.exists(os.path.join(target, upload_name)))446 self.assertTrue(os.path.exists(os.path.join(target, upload_name)))
@@ -455,7 +461,7 @@
455 # Remove it461 # Remove it
456 self.options.base_fsroot = testdir462 self.options.base_fsroot = testdir
457 up = self.getUploadProcessor(None)463 up = self.getUploadProcessor(None)
458 up.moveProcessedUpload(upload, "accepted")464 up.moveProcessedUpload(upload, "accepted", self.log)
459465
460 # Check it was removed, not moved466 # Check it was removed, not moved
461 self.assertFalse(os.path.exists(os.path.join(467 self.assertFalse(os.path.exists(os.path.join(
@@ -478,7 +484,7 @@
478 # Move it484 # Move it
479 self.options.base_fsroot = testdir485 self.options.base_fsroot = testdir
480 up = self.getUploadProcessor(None)486 up = self.getUploadProcessor(None)
481 up.moveProcessedUpload(upload, "rejected")487 up.moveProcessedUpload(upload, "rejected", self.log)
482488
483 # Check it moved489 # Check it moved
484 self.assertTrue(os.path.exists(os.path.join(testdir,490 self.assertTrue(os.path.exists(os.path.join(testdir,
@@ -501,7 +507,7 @@
501 # Remove it507 # Remove it
502 self.options.base_fsroot = testdir508 self.options.base_fsroot = testdir
503 up = self.getUploadProcessor(None)509 up = self.getUploadProcessor(None)
504 up.removeUpload(upload)510 up.removeUpload(upload, self.log)
505511
506 # Check it was removed, not moved512 # Check it was removed, not moved
507 self.assertFalse(os.path.exists(os.path.join(513 self.assertFalse(os.path.exists(os.path.join(
@@ -1316,6 +1322,7 @@
1316 used.1322 used.
1317 That exception will then initiate the creation of an OOPS report.1323 That exception will then initiate the creation of an OOPS report.
1318 """1324 """
1325 self.options.builds = False
1319 processor = self.getUploadProcessor(self.layer.txn)1326 processor = self.getUploadProcessor(self.layer.txn)
13201327
1321 upload_dir = self.queueUpload("foocomm_1.0-1_proposed")1328 upload_dir = self.queueUpload("foocomm_1.0-1_proposed")
@@ -1825,3 +1832,101 @@
1825 pubrec.datepublished = UTC_NOW1832 pubrec.datepublished = UTC_NOW
1826 queue_item.setDone()1833 queue_item.setDone()
1827 self.PGPSignatureNotPreserved(archive=self.breezy.main_archive)1834 self.PGPSignatureNotPreserved(archive=self.breezy.main_archive)
1835
1836
1837class TestBuildUploadProcessor(TestUploadProcessorBase):
1838 """Test that processing build uploads works."""
1839
1840 def setUp(self):
1841 super(TestBuildUploadProcessor, self).setUp()
1842 self.uploadprocessor = self.setupBreezyAndGetUploadProcessor()
1843
1844 def testInvalidLeafName(self):
1845 upload_dir = self.queueUpload("bar_1.0-1")
1846 self.uploadprocessor.processBuildUpload(upload_dir, "bar_1.0-1")
1847 self.assertLogContains('Unable to extract build id from leaf '
1848 'name bar_1.0-1, skipping.')
1849
1850 def testNoBuildEntry(self):
1851 upload_dir = self.queueUpload("bar_1.0-1", queue_entry="42-60")
1852 self.assertRaises(NotFoundError, self.uploadprocessor.processBuildUpload,
1853 upload_dir, "42-60")
1854
1855 def testNoFiles(self):
1856 # Upload a source package
1857 upload_dir = self.queueUpload("bar_1.0-1")
1858 self.processUpload(self.uploadprocessor, upload_dir)
1859 source_pub = self.publishPackage('bar', '1.0-1')
1860 [build] = source_pub.createMissingBuilds()
1861
1862 # Move the source from the accepted queue.
1863 [queue_item] = self.breezy.getQueueItems(
1864 status=PackageUploadStatus.ACCEPTED,
1865 version="1.0-1", name="bar")
1866 queue_item.setDone()
1867
1868 build.jobStarted()
1869 build.builder = self.factory.makeBuilder()
1870
1871 # Upload and accept a binary for the primary archive source.
1872 shutil.rmtree(upload_dir)
1873 self.layer.txn.commit()
1874 leaf_name = "%d-%d" % (build.id, 60)
1875 os.mkdir(os.path.join(self.incoming_folder, leaf_name))
1876 self.options.context = 'buildd'
1877 self.options.builds = True
1878 self.uploadprocessor.processBuildUpload(self.incoming_folder, leaf_name)
1879 self.layer.txn.commit()
1880 self.assertEquals(BuildStatus.FAILEDTOUPLOAD, build.status)
1881 log_contents = build.upload_log.read()
1882 self.assertTrue('ERROR: Exception while processing upload '
1883 in log_contents)
1884 self.assertTrue('DEBUG: Moving upload directory '
1885 in log_contents)
1886
1887 def testSuccess(self):
1888 # Upload a source package
1889 upload_dir = self.queueUpload("bar_1.0-1")
1890 self.processUpload(self.uploadprocessor, upload_dir)
1891 source_pub = self.publishPackage('bar', '1.0-1')
1892 [build] = source_pub.createMissingBuilds()
1893
1894 # Move the source from the accepted queue.
1895 [queue_item] = self.breezy.getQueueItems(
1896 status=PackageUploadStatus.ACCEPTED,
1897 version="1.0-1", name="bar")
1898 queue_item.setDone()
1899
1900 build.jobStarted()
1901 build.builder = self.factory.makeBuilder()
1902
1903 # Upload and accept a binary for the primary archive source.
1904 shutil.rmtree(upload_dir)
1905 self.layer.txn.commit()
1906 leaf_name = "%d-%d" % (build.id, 60)
1907 upload_dir = self.queueUpload("bar_1.0-1_binary",
1908 queue_entry=leaf_name)
1909 self.options.context = 'buildd'
1910 self.options.builds = True
1911 self.uploadprocessor.processBuildUpload(self.incoming_folder, leaf_name)
1912 self.layer.txn.commit()
1913 self.assertEquals(BuildStatus.FULLYBUILT, build.status)
1914 log_lines = build.upload_log.read().splitlines()
1915 self.assertTrue(
1916 'INFO: Processing upload bar_1.0-1_i386.changes' in log_lines)
1917 self.assertTrue(
1918 'INFO: Committing the transaction and any mails associated with '
1919 'this upload.' in log_lines)
1920
1921
1922class ParseBuildUploadLeafNameTests(TestCase):
1923 """Tests for parse_build_upload_leaf_name."""
1924
1925 def test_valid(self):
1926 self.assertEquals((42, 60), parse_build_upload_leaf_name("42-60"))
1927
1928 def test_invalid_chars(self):
1929 self.assertRaises(ValueError, parse_build_upload_leaf_name, "a42-460")
1930
1931 def test_no_dash(self):
1932 self.assertRaises(ValueError, parse_build_upload_leaf_name, "32")
18281933
=== modified file 'lib/lp/archiveuploader/uploadpolicy.py'
--- lib/lp/archiveuploader/uploadpolicy.py 2010-08-18 16:12:28 +0000
+++ lib/lp/archiveuploader/uploadpolicy.py 2010-08-19 14:33:31 +0000
@@ -290,7 +290,8 @@
290 def setOptions(self, options):290 def setOptions(self, options):
291 AbstractUploadPolicy.setOptions(self, options)291 AbstractUploadPolicy.setOptions(self, options)
292 # We require a buildid to be provided292 # We require a buildid to be provided
293 if getattr(options, 'buildid', None) is None:293 if (getattr(options, 'buildid', None) is None and
294 not getattr(options, 'builds', False)):
294 raise UploadPolicyError("BuildID required for buildd context")295 raise UploadPolicyError("BuildID required for buildd context")
295296
296 def policySpecificChecks(self, upload):297 def policySpecificChecks(self, upload):
297298
=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py 2010-08-17 18:33:26 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py 2010-08-19 14:33:31 +0000
@@ -47,7 +47,9 @@
4747
48__metaclass__ = type48__metaclass__ = type
4949
50import datetime
50import os51import os
52import pytz
51import shutil53import shutil
52import stat54import stat
53import sys55import sys
@@ -63,9 +65,12 @@
63 BuildDaemonUploadPolicy,65 BuildDaemonUploadPolicy,
64 SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME,66 SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME,
65 UploadPolicyError)67 UploadPolicyError)
68from lp.buildmaster.interfaces.buildbase import BuildStatus
66from lp.soyuz.interfaces.archive import IArchiveSet, NoSuchPPA69from lp.soyuz.interfaces.archive import IArchiveSet, NoSuchPPA
70from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
67from lp.registry.interfaces.distribution import IDistributionSet71from lp.registry.interfaces.distribution import IDistributionSet
68from lp.registry.interfaces.person import IPersonSet72from lp.registry.interfaces.person import IPersonSet
73from canonical.launchpad.scripts.logger import BufferLogger
69from canonical.launchpad.webapp.errorlog import (74from canonical.launchpad.webapp.errorlog import (
70 ErrorReportingUtility, ScriptRequest)75 ErrorReportingUtility, ScriptRequest)
7176
@@ -73,6 +78,7 @@
7378
74__all__ = [79__all__ = [
75 'UploadProcessor',80 'UploadProcessor',
81 'parse_build_upload_leaf_name',
76 'parse_upload_path',82 'parse_upload_path',
77 ]83 ]
7884
@@ -86,6 +92,19 @@
86""")92""")
8793
8894
95def parse_build_upload_leaf_name(name):
96 """Parse the leaf directory name of a build upload.
97
98 :param name: Directory name.
99 :return: Tuple with build id and build queue record id.
100 """
101 (build_id_str, queue_record_str) = name.split("-", 1)
102 try:
103 return int(build_id_str), int(queue_record_str)
104 except TypeError:
105 raise ValueError
106
107
89class UploadStatusEnum:108class UploadStatusEnum:
90 """Possible results from processing an upload.109 """Possible results from processing an upload.
91110
@@ -110,8 +129,8 @@
110class UploadProcessor:129class UploadProcessor:
111 """Responsible for processing uploads. See module docstring."""130 """Responsible for processing uploads. See module docstring."""
112131
113 def __init__(self, base_fsroot, dry_run, no_mails, keep, policy_for_distro,132 def __init__(self, base_fsroot, dry_run, no_mails, builds, keep,
114 ztm, log):133 policy_for_distro, ztm, log):
115 """Create a new upload processor.134 """Create a new upload processor.
116135
117 :param base_fsroot: Root path for queue to use136 :param base_fsroot: Root path for queue to use
@@ -130,6 +149,7 @@
130 self.last_processed_upload = None149 self.last_processed_upload = None
131 self.log = log150 self.log = log
132 self.no_mails = no_mails151 self.no_mails = no_mails
152 self.builds = builds
133 self._getPolicyForDistro = policy_for_distro153 self._getPolicyForDistro = policy_for_distro
134 self.ztm = ztm154 self.ztm = ztm
135155
@@ -161,11 +181,63 @@
161 self.log.debug("Skipping %s -- does not match %s" % (181 self.log.debug("Skipping %s -- does not match %s" % (
162 upload, leaf_name))182 upload, leaf_name))
163 continue183 continue
164 self.processUpload(fsroot, upload)184 if self.builds:
185 # Upload directories contain build results,
186 # directories are named after build ids.
187 self.processBuildUpload(fsroot, upload)
188 else:
189 self.processUpload(fsroot, upload)
165 finally:190 finally:
166 self.log.debug("Rolling back any remaining transactions.")191 self.log.debug("Rolling back any remaining transactions.")
167 self.ztm.abort()192 self.ztm.abort()
168193
194 def processBuildUpload(self, fsroot, upload):
195 """Process an upload that is the result of a build.
196
197 The name of the leaf is the build id of the build.
198 Build uploads always contain a single package per leaf.
199 """
200 try:
201 (build_id, build_queue_record_id) = parse_build_upload_leaf_name(
202 upload)
203 except ValueError:
204 self.log.warn("Unable to extract build id from leaf name %s,"
205 " skipping." % upload)
206 return
207 build = getUtility(IBinaryPackageBuildSet).getByBuildID(int(build_id))
208 self.log.debug("Build %s found" % build.id)
209 logger = BufferLogger()
210 upload_path = os.path.join(fsroot, upload)
211 try:
212 [changes_file] = self.locateChangesFiles(upload_path)
213 logger.debug("Considering changefile %s" % changes_file)
214 result = self.processChangesFile(
215 upload_path, changes_file, logger)
216 except (KeyboardInterrupt, SystemExit):
217 raise
218 except:
219 info = sys.exc_info()
220 message = (
221 'Exception while processing upload %s' % upload_path)
222 properties = [('error-explanation', message)]
223 request = ScriptRequest(properties)
224 error_utility = ErrorReportingUtility()
225 error_utility.raising(info, request)
226 logger.error('%s (%s)' % (message, request.oopsid))
227 result = UploadStatusEnum.FAILED
228 destination = {
229 UploadStatusEnum.FAILED: "failed",
230 UploadStatusEnum.REJECTED: "rejected",
231 UploadStatusEnum.ACCEPTED: "accepted"}[result]
232 self.moveProcessedUpload(upload_path, destination, logger)
233 if not (result == UploadStatusEnum.ACCEPTED and
234 build.verifySuccessfulUpload() and
235 build.status == BuildStatus.FULLYBUILT):
236 build.status = BuildStatus.FAILEDTOUPLOAD
237 build.date_finished = datetime.datetime.now(pytz.UTC)
238 build.notify(extra_info="Uploading build %s failed." % upload)
239 build.storeUploadLog(logger.buffer.getvalue())
240
169 def processUpload(self, fsroot, upload):241 def processUpload(self, fsroot, upload):
170 """Process an upload's changes files, and move it to a new directory.242 """Process an upload's changes files, and move it to a new directory.
171243
@@ -186,7 +258,8 @@
186 for changes_file in changes_files:258 for changes_file in changes_files:
187 self.log.debug("Considering changefile %s" % changes_file)259 self.log.debug("Considering changefile %s" % changes_file)
188 try:260 try:
189 result = self.processChangesFile(upload_path, changes_file)261 result = self.processChangesFile(
262 upload_path, changes_file, self.log)
190 if result == UploadStatusEnum.FAILED:263 if result == UploadStatusEnum.FAILED:
191 some_failed = True264 some_failed = True
192 elif result == UploadStatusEnum.REJECTED:265 elif result == UploadStatusEnum.REJECTED:
@@ -216,8 +289,7 @@
216 # There were no changes files at all. We consider289 # There were no changes files at all. We consider
217 # the upload to be failed in this case.290 # the upload to be failed in this case.
218 destination = "failed"291 destination = "failed"
219292 self.moveProcessedUpload(upload_path, destination, self.log)
220 self.moveProcessedUpload(upload_path, destination)
221293
222 def locateDirectories(self, fsroot):294 def locateDirectories(self, fsroot):
223 """Return a list of upload directories in a given queue.295 """Return a list of upload directories in a given queue.
@@ -280,7 +352,7 @@
280 os.path.join(relative_path, filename))352 os.path.join(relative_path, filename))
281 return self.orderFilenames(changes_files)353 return self.orderFilenames(changes_files)
282354
283 def processChangesFile(self, upload_path, changes_file):355 def processChangesFile(self, upload_path, changes_file, logger=None):
284 """Process a single changes file.356 """Process a single changes file.
285357
286 This is done by obtaining the appropriate upload policy (according358 This is done by obtaining the appropriate upload policy (according
@@ -297,6 +369,8 @@
297 Returns a value from UploadStatusEnum, or re-raises an exception369 Returns a value from UploadStatusEnum, or re-raises an exception
298 from NascentUpload.370 from NascentUpload.
299 """371 """
372 if logger is None:
373 logger = self.log
300 # Calculate the distribution from the path within the upload374 # Calculate the distribution from the path within the upload
301 # Reject the upload since we could not process the path,375 # Reject the upload since we could not process the path,
302 # Store the exception information as a rejection message.376 # Store the exception information as a rejection message.
@@ -333,7 +407,7 @@
333 "Please check the documentation at "407 "Please check the documentation at "
334 "https://help.launchpad.net/Packaging/PPA#Uploading "408 "https://help.launchpad.net/Packaging/PPA#Uploading "
335 "and update your configuration.")))409 "and update your configuration.")))
336 self.log.debug("Finding fresh policy")410 logger.debug("Finding fresh policy")
337 policy = self._getPolicyForDistro(distribution)411 policy = self._getPolicyForDistro(distribution)
338 policy.archive = archive412 policy.archive = archive
339413
@@ -360,7 +434,7 @@
360 "Invalid upload path (%s) for this policy (%s)" %434 "Invalid upload path (%s) for this policy (%s)" %
361 (relative_path, policy.name))435 (relative_path, policy.name))
362 upload.reject(error_message)436 upload.reject(error_message)
363 self.log.error(error_message)437 logger.error(error_message)
364438
365 # Reject upload with path processing errors.439 # Reject upload with path processing errors.
366 if upload_path_error is not None:440 if upload_path_error is not None:
@@ -370,7 +444,7 @@
370 self.last_processed_upload = upload444 self.last_processed_upload = upload
371445
372 try:446 try:
373 self.log.info("Processing upload %s" % upload.changes.filename)447 logger.info("Processing upload %s" % upload.changes.filename)
374 result = UploadStatusEnum.ACCEPTED448 result = UploadStatusEnum.ACCEPTED
375449
376 try:450 try:
@@ -378,12 +452,12 @@
378 except UploadPolicyError, e:452 except UploadPolicyError, e:
379 upload.reject("UploadPolicyError escaped upload.process: "453 upload.reject("UploadPolicyError escaped upload.process: "
380 "%s " % e)454 "%s " % e)
381 self.log.debug("UploadPolicyError escaped upload.process",455 logger.debug(
382 exc_info=True)456 "UploadPolicyError escaped upload.process", exc_info=True)
383 except FatalUploadError, e:457 except FatalUploadError, e:
384 upload.reject("UploadError escaped upload.process: %s" % e)458 upload.reject("UploadError escaped upload.process: %s" % e)
385 self.log.debug("UploadError escaped upload.process",459 logger.debug(
386 exc_info=True)460 "UploadError escaped upload.process", exc_info=True)
387 except (KeyboardInterrupt, SystemExit):461 except (KeyboardInterrupt, SystemExit):
388 raise462 raise
389 except EarlyReturnUploadError:463 except EarlyReturnUploadError:
@@ -400,7 +474,7 @@
400 # the new exception will be handled by the caller just like474 # the new exception will be handled by the caller just like
401 # the one we caught would have been, by failing the upload475 # the one we caught would have been, by failing the upload
402 # with no email.476 # with no email.
403 self.log.exception("Unhandled exception processing upload")477 logger.exception("Unhandled exception processing upload")
404 upload.reject("Unhandled exception processing upload: %s" % e)478 upload.reject("Unhandled exception processing upload: %s" % e)
405479
406 # XXX julian 2007-05-25 bug=29744:480 # XXX julian 2007-05-25 bug=29744:
@@ -418,21 +492,22 @@
418 successful = upload.do_accept(notify=notify)492 successful = upload.do_accept(notify=notify)
419 if not successful:493 if not successful:
420 result = UploadStatusEnum.REJECTED494 result = UploadStatusEnum.REJECTED
421 self.log.info("Rejection during accept. "495 logger.info(
422 "Aborting partial accept.")496 "Rejection during accept. Aborting partial accept.")
423 self.ztm.abort()497 self.ztm.abort()
424498
425 if upload.is_rejected:499 if upload.is_rejected:
426 self.log.warn("Upload was rejected:")500 logger.warn("Upload was rejected:")
427 for msg in upload.rejections:501 for msg in upload.rejections:
428 self.log.warn("\t%s" % msg)502 logger.warn("\t%s" % msg)
429503
430 if self.dry_run:504 if self.dry_run:
431 self.log.info("Dry run, aborting transaction.")505 logger.info("Dry run, aborting transaction.")
432 self.ztm.abort()506 self.ztm.abort()
433 else:507 else:
434 self.log.info("Committing the transaction and any mails "508 logger.info(
435 "associated with this upload.")509 "Committing the transaction and any mails associated "
510 "with this upload.")
436 self.ztm.commit()511 self.ztm.commit()
437 except:512 except:
438 self.ztm.abort()513 self.ztm.abort()
@@ -440,47 +515,47 @@
440515
441 return result516 return result
442517
443 def removeUpload(self, upload):518 def removeUpload(self, upload, logger):
444 """Remove an upload that has succesfully been processed.519 """Remove an upload that has succesfully been processed.
445520
446 This includes moving the given upload directory and moving the521 This includes moving the given upload directory and moving the
447 matching .distro file, if it exists.522 matching .distro file, if it exists.
448 """523 """
449 if self.keep or self.dry_run:524 if self.keep or self.dry_run:
450 self.log.debug("Keeping contents untouched")525 logger.debug("Keeping contents untouched")
451 return526 return
452527
453 self.log.debug("Removing upload directory %s", upload)528 logger.debug("Removing upload directory %s", upload)
454 shutil.rmtree(upload)529 shutil.rmtree(upload)
455530
456 distro_filename = upload + ".distro"531 distro_filename = upload + ".distro"
457 if os.path.isfile(distro_filename):532 if os.path.isfile(distro_filename):
458 self.log.debug("Removing distro file %s", distro_filename)533 logger.debug("Removing distro file %s", distro_filename)
459 os.remove(distro_filename)534 os.remove(distro_filename)
460535
461 def moveProcessedUpload(self, upload_path, destination):536 def moveProcessedUpload(self, upload_path, destination, logger):
462 """Move or remove the upload depending on the status of the upload.537 """Move or remove the upload depending on the status of the upload.
463 """538 """
464 if destination == "accepted":539 if destination == "accepted":
465 self.removeUpload(upload_path)540 self.removeUpload(upload_path, logger)
466 else:541 else:
467 self.moveUpload(upload_path, destination)542 self.moveUpload(upload_path, destination, logger)
468543
469 def moveUpload(self, upload, subdir_name):544 def moveUpload(self, upload, subdir_name, logger):
470 """Move the upload to the named subdir of the root, eg 'accepted'.545 """Move the upload to the named subdir of the root, eg 'accepted'.
471546
472 This includes moving the given upload directory and moving the547 This includes moving the given upload directory and moving the
473 matching .distro file, if it exists.548 matching .distro file, if it exists.
474 """549 """
475 if self.keep or self.dry_run:550 if self.keep or self.dry_run:
476 self.log.debug("Keeping contents untouched")551 logger.debug("Keeping contents untouched")
477 return552 return
478553
479 pathname = os.path.basename(upload)554 pathname = os.path.basename(upload)
480555
481 target_path = os.path.join(556 target_path = os.path.join(
482 self.base_fsroot, subdir_name, pathname)557 self.base_fsroot, subdir_name, pathname)
483 self.log.debug("Moving upload directory %s to %s" %558 logger.debug("Moving upload directory %s to %s" %
484 (upload, target_path))559 (upload, target_path))
485 shutil.move(upload, target_path)560 shutil.move(upload, target_path)
486561
@@ -488,7 +563,7 @@
488 if os.path.isfile(distro_filename):563 if os.path.isfile(distro_filename):
489 target_path = os.path.join(self.base_fsroot, subdir_name,564 target_path = os.path.join(self.base_fsroot, subdir_name,
490 os.path.basename(distro_filename))565 os.path.basename(distro_filename))
491 self.log.debug("Moving distro file %s to %s" % (distro_filename,566 logger.debug("Moving distro file %s to %s" % (distro_filename,
492 target_path))567 target_path))
493 shutil.move(distro_filename, target_path)568 shutil.move(distro_filename, target_path)
494569
495570
=== modified file 'lib/lp/soyuz/scripts/soyuz_process_upload.py'
--- lib/lp/soyuz/scripts/soyuz_process_upload.py 2010-08-18 14:03:15 +0000
+++ lib/lp/soyuz/scripts/soyuz_process_upload.py 2010-08-19 14:33:31 +0000
@@ -35,6 +35,11 @@
35 help="Whether to suppress the sending of mails or not.")35 help="Whether to suppress the sending of mails or not.")
3636
37 self.parser.add_option(37 self.parser.add_option(
38 "--builds", action="store_true",
39 dest="builds", default=False,
40 help="Whether to interpret leaf names as build ids.")
41
42 self.parser.add_option(
38 "-J", "--just-leaf", action="store", dest="leafname",43 "-J", "--just-leaf", action="store", dest="leafname",
39 default=None, help="A specific leaf dir to limit to.",44 default=None, help="A specific leaf dir to limit to.",
40 metavar = "LEAF")45 metavar = "LEAF")
@@ -80,9 +85,9 @@
80 policy = findPolicyByName(self.options.context)85 policy = findPolicyByName(self.options.context)
81 policy.setOptions(self.options)86 policy.setOptions(self.options)
82 return policy87 return policy
83 processor = UploadProcessor(self.options.base_fsroot, 88 processor = UploadProcessor(self.options.base_fsroot,
84 self.options.dryrun, self.options.nomails, self.options.keep,89 self.options.dryrun, self.options.nomails, self.options.builds,
85 getPolicy, self.txn, self.logger)90 self.options.keep, getPolicy, self.txn, self.logger)
86 processor.processUploadQueue(self.options.leafname)91 processor.processUploadQueue(self.options.leafname)
8792
88 @property93 @property