launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/webhook-jobrunner into lp:launchpad with lp:~wgrant/launchpad/webhook-api as a prerequisite.

Commit message:
Fix and test cron and celery WebhookDeliveryJob running.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/webhook-jobrunner/+merge/264533

Some quick fixes and tests for running WebhookDeliveryJobs from the cronscript and celery.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/webhook-jobrunner into lp:launchpad.

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf	2015-07-13 11:01:57 +0000
+++ configs/testrunner/launchpad-lazr.conf	2015-07-13 11:01:58 +0000
@@ -182,4 +182,4 @@
 root: /var/tmp/testkeyserver.test
 
 [webhooks]
-http_proxy: http://example.com:3128/
+http_proxy: http://webhooks-proxy.invalid:3128/

=== modified file 'lib/lp/services/webhooks/configure.zcml'
--- lib/lp/services/webhooks/configure.zcml	2015-07-13 11:01:57 +0000
+++ lib/lp/services/webhooks/configure.zcml	2015-07-13 11:01:58 +0000
@@ -16,19 +16,23 @@
     <subscriber
         for="lp.services.webhooks.interfaces.IWebhook zope.lifecycleevent.interfaces.IObjectModifiedEvent"
         handler="lp.services.webhooks.model.webhook_modified"/>
-
-    <class class="lp.services.webhooks.model.WebhookDeliveryJob">
-        <require
-            permission="launchpad.View"
-            interface="lp.services.webhooks.interfaces.IWebhookDeliveryJob"/>
-    </class>
-
     <securedutility
         class="lp.services.webhooks.model.WebhookSource"
         provides="lp.services.webhooks.interfaces.IWebhookSource">
         <allow interface="lp.services.webhooks.interfaces.IWebhookSource"/>
     </securedutility>
 
+    <securedutility
+        component="lp.services.webhooks.model.WebhookDeliveryJob"
+        provides="lp.services.webhooks.interfaces.IWebhookDeliveryJobSource">
+        <allow interface="lp.services.webhooks.interfaces.IWebhookDeliveryJobSource"/>
+    </securedutility>
+    <class class="lp.services.webhooks.model.WebhookDeliveryJob">
+        <require
+            permission="launchpad.View"
+            interface="lp.services.webhooks.interfaces.IWebhookDeliveryJob"/>
+    </class>
+
     <utility
         provides="lp.services.webhooks.interfaces.IWebhookClient"
         factory="lp.services.webhooks.client.WebhookClient"

=== modified file 'lib/lp/services/webhooks/model.py'
--- lib/lp/services/webhooks/model.py	2015-07-13 11:01:57 +0000
+++ lib/lp/services/webhooks/model.py	2015-07-13 11:01:58 +0000
@@ -41,7 +41,10 @@
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import EnumCol
-from lp.services.database.interfaces import IStore
+from lp.services.database.interfaces import (
+    IMasterStore,
+    IStore,
+    )
 from lp.services.database.stormbase import StormBase
 from lp.services.features import getFeatureFlag
 from lp.services.job.model.job import (
@@ -242,6 +245,16 @@
     def __init__(self, webhook_job):
         self.context = webhook_job
 
+    @classmethod
+    def iterReady(cls):
+        """See `IJobSource`."""
+        jobs = IMasterStore(WebhookJob).find(
+            WebhookJob,
+            WebhookJob.job_type == cls.class_job_type,
+            WebhookJob.job == Job.id,
+            Job.id.is_in(Job.ready_jobs))
+        return (cls(job) for job in jobs)
+
 
 @provider(IWebhookDeliveryJobSource)
 @implementer(IWebhookDeliveryJob)

=== modified file 'lib/lp/services/webhooks/tests/test_webhookjob.py'
--- lib/lp/services/webhooks/tests/test_webhookjob.py	2015-07-13 11:01:57 +0000
+++ lib/lp/services/webhooks/tests/test_webhookjob.py	2015-07-13 11:01:58 +0000
@@ -21,9 +21,13 @@
     MatchesStructure,
     Not,
     )
+import transaction
 
+from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.runner import JobRunner
+from lp.services.job.tests import block_on_job
+from lp.services.scripts.tests import run_script
 from lp.services.webhooks.client import WebhookClient
 from lp.services.webhooks.interfaces import (
     IWebhookClient,
@@ -43,8 +47,9 @@
     ZopeUtilityFixture,
     )
 from lp.testing.layers import (
+    CeleryJobLayer,
     DatabaseFunctionalLayer,
-    LaunchpadZopelessLayer,
+    ZopelessDatabaseLayer,
     )
 
 
@@ -63,7 +68,7 @@
 class TestWebhookJobDerived(TestCaseWithFactory):
     """Tests for `WebhookJobDerived`."""
 
-    layer = LaunchpadZopelessLayer
+    layer = DatabaseFunctionalLayer
 
     def test_getOopsMailController(self):
         """By default, no mail is sent about failed WebhookJobs."""
@@ -147,7 +152,7 @@
 class TestWebhookDeliveryJob(TestCaseWithFactory):
     """Tests for `WebhookDeliveryJob`."""
 
-    layer = LaunchpadZopelessLayer
+    layer = ZopelessDatabaseLayer
 
     def makeAndRunJob(self, response_status=200, raises=None, mock=True):
         hook = self.factory.makeWebhook(delivery_url=u'http://hookep.com/foo')
@@ -251,3 +256,45 @@
         self.assertEqual(1, len(oopses.oopses))
         self.assertEqual(
             'No webhook proxy configured.', oopses.oopses[0]['value'])
+
+
+class TestViaCronscript(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def test_run_from_cronscript(self):
+        hook = self.factory.makeWebhook(delivery_url=u'http://hookep.com/foo')
+        job = WebhookDeliveryJob.create(hook, payload={'foo': 'bar'})
+        self.assertEqual(JobStatus.WAITING, job.status)
+        transaction.commit()
+
+        retcode, stdout, stderr = run_script(
+            'cronscripts/process-job-source.py', ['IWebhookDeliveryJobSource'],
+            expect_returncode=0)
+        self.assertEqual('', stdout)
+        self.assertIn('INFO    Ran 1 WebhookDeliveryJob jobs.\n', stderr)
+
+        self.assertEqual(JobStatus.COMPLETED, job.status)
+        self.assertIn(
+            'Cannot connect to proxy',
+            job.json_data['result']['connection_error'])
+
+
+class TestViaCelery(TestCaseWithFactory):
+
+    layer = CeleryJobLayer
+
+    def test_WebhookDeliveryJob(self):
+        """MergeProposalNeedsReviewEmailJob runs under Celery."""
+        hook = self.factory.makeWebhook(delivery_url=u'http://hookep.com/foo')
+
+        self.useFixture(FeatureFixture(
+            {'jobs.celery.enabled_classes': 'WebhookDeliveryJob'}))
+        with block_on_job():
+            job = WebhookDeliveryJob.create(hook, payload={'foo': 'bar'})
+            transaction.commit()
+
+        self.assertEqual(JobStatus.COMPLETED, job.status)
+        self.assertIn(
+            'Cannot connect to proxy',
+            job.json_data['result']['connection_error'])