Code review comment for lp:~gmb/launchpad/blobjob-cronscript-bug-513190

Non-crazy diff:

=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2010-01-22 04:01:17 +0000
+++ configs/development/launchpad-lazr.conf 2010-02-12 15:44:24 +0000
@@ -221,6 +221,9 @@
 error_dir: /var/tmp/poimport
 oops_prefix: POI

+[process_apport_blobs]
+error_dir: /var/tmp/lperr
+
 [supermirror_puller]
 error_dir: /var/tmp/codehosting.test
 oops_prefix: SMP

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf 2010-01-29 17:15:31 +0000
+++ configs/testrunner/launchpad-lazr.conf 2010-02-12 15:44:24 +0000
@@ -193,6 +193,11 @@
 error_dir: /var/tmp/poimport.test
 oops_prefix: TPOI

+[process_apport_blobs]
+dbuser: process-apport-blobs
+oops_prefix: TAPPORTBLOB
+error_dir: /var/tmp/lperr.test
+
 [rosettabranches]
 oops_prefix: TRSBR
 error_dir: /var/tmp/rosettabranches.test

=== added file 'cronscripts/process-apport-blobs.py'
--- cronscripts/process-apport-blobs.py 1970-01-01 00:00:00 +0000
+++ cronscripts/process-apport-blobs.py 2010-02-11 18:50:58 +0000
@@ -0,0 +1,33 @@
+#!/usr/bin/python2.5
+#
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# pylint: disable-msg=W0403
+
+"""Process uploaded Apport BLOBs."""
+
+__metaclass__ = type
+
+import _pythonpath
+
+from canonical.launchpad.webapp import errorlog
+
+from lp.services.job.runner import JobCronScript
+from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
+
+
+class RunProcessApportBlobs(JobCronScript):
+ """Run ProcessApportBlobJobs."""
+
+ config_name = 'process_apport_blobs'
+ source_interface = IProcessApportBlobJobSource
+
+ def main(self):
+ errorlog.globalErrorUtility.configure(self.config_name)
+ return super(RunProcessApportBlobs, self).main()
+
+
+if __name__ == '__main__':
+ script = RunProcessApportBlobs()
+ script.lock_and_run()

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2010-02-02 11:58:46 +0000
+++ database/schema/security.cfg 2010-02-12 15:44:24 +0000
@@ -1897,3 +1897,11 @@
 public.bug = SELECT, UPDATE
 public.job = SELECT, UPDATE, DELETE
 public.bugjob = SELECT, DELETE
+
+[process-apport-blobs]
+type=user
+groups=script,read
+public.job = SELECT, UPDATE, DELETE
+public.apportjob = SELECT, INSERT, UPDATE, DELETE
+public.libraryfilealias = SELECT, INSERT, UPDATE
+public.libraryfilecontent = SELECT, INSERT, UPDATE

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf 2010-01-22 04:01:17 +0000
+++ lib/canonical/config/schema-lazr.conf 2010-02-12 15:44:24 +0000
@@ -28,7 +28,6 @@
 # datatype: string
 base_url: http://ftpmaster.internal/

-
 [calculate_bug_heat]
 # The database user which will be used by this process.
 # datatype: string
@@ -37,6 +36,13 @@
 error_dir: none
 copy_to_zlog: false

+[process_apport_blobs]
+# The database user which will be used by this process.
+# datatype: string
+dbuser: process-apport-blobs
+oops_prefix: APPORTBLOB
+error_dir: none
+copy_to_zlog: false

 [branchscanner]
 # The database user which will be used by this process.

=== modified file 'lib/canonical/launchpad/browser/temporaryblobstorage.py'
--- lib/canonical/launchpad/browser/temporaryblobstorage.py 2010-02-10 14:17:10 +0000
+++ lib/canonical/launchpad/browser/temporaryblobstorage.py 2010-02-12 12:16:54 +0000
@@ -43,9 +43,10 @@
     @action('Continue', name='FORM_SUBMIT')
     def continue_action(self, action, data):
         uuid = self.store_blob(data['blob'])
- self.request.response.setHeader('X-Launchpad-Blob-Token', uuid)
- self.request.response.addInfoNotification(
- 'Your ticket is "%s"' % uuid)
+ if uuid is not None:
+ self.request.response.setHeader('X-Launchpad-Blob-Token', uuid)
+ self.request.response.addInfoNotification(
+ 'Your ticket is "%s"' % uuid)

     def store_blob(self, blob):
         """Store a blob and return its UUID."""
@@ -53,10 +54,13 @@
             uuid = getUtility(ITemporaryStorageManager).new(blob)
         except BlobTooLarge:
             self.addError('Uploaded file was too large.')
- except UploadFailed:
+ return None
+ except UploadFailed, e:
+ import pdb; pdb.set_trace()
             self.addError('File storage unavailable - try again later.')
-
- # Create ProcessApportBlobJob for the BLOB.
- blob = getUtility(ITemporaryStorageManager).fetch(uuid)
- getUtility(IProcessApportBlobJobSource).create(blob)
- return uuid
+ return None
+ else:
+ # Create ProcessApportBlobJob for the BLOB.
+ blob = getUtility(ITemporaryStorageManager).fetch(uuid)
+ getUtility(IProcessApportBlobJobSource).create(blob)
+ return uuid

=== modified file 'lib/lp/bugs/tests/test_apportjob.py'
--- lib/lp/bugs/tests/test_apportjob.py 2010-02-11 10:59:08 +0000
+++ lib/lp/bugs/tests/test_apportjob.py 2010-02-12 15:44:24 +0000
@@ -16,6 +16,7 @@
 from canonical.launchpad.interfaces.temporaryblobstorage import (
     ITemporaryStorageManager)
 from canonical.launchpad.webapp.interfaces import ILaunchpadRoot
+from canonical.launchpad.scripts.tests import run_script
 from canonical.testing import (
     LaunchpadFunctionalLayer, LaunchpadZopelessLayer)

@@ -224,6 +225,19 @@
         # it's attached to the same BLOB.
         self.assertEqual(job.id, yet_another_job.id, "Jobs do not match.")

+ def test_cronscript_succeeds(self):
+ # The process-apport-blobs cronscript will run all pending
+ # ProcessApportBlobJobs.
+ ProcessApportBlobJob.create(self.blob)
+ transaction.commit()
+
+ retcode, stdout, stderr = run_script(
+ 'cronscripts/process-apport-blobs.py', [],
+ expect_returncode=0)
+ self.assertEqual('', stdout)
+ self.assertIn(
+ 'INFO Ran 1 IProcessApportBlobJobSource jobs.\n', stderr)
+

 class TestTemporaryBlobStorageAddView(TestCaseWithFactory):
     """Test case for the TemporaryBlobStorageAddView."""