launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/webhook-job-ordering into lp:launchpad.

Commit message:
Order webhook delivery jobs by job ID.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/webhook-job-ordering/+merge/321188

The remote end can't make any assumptions about delivery ordering (as I've now noted in https://help.launchpad.net/API/Webhooks#Delivery_ordering), but it's still a bit friendlier and less confusing to make an effort to deliver webhooks in the order that they were created.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/webhook-job-ordering into lp:launchpad.

=== modified file 'lib/lp/services/webhooks/model.py'
--- lib/lp/services/webhooks/model.py	2016-09-14 11:13:06 +0000
+++ lib/lp/services/webhooks/model.py	2017-03-28 15:34:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -366,7 +366,7 @@
             WebhookJob,
             WebhookJob.job_type == cls.class_job_type,
             WebhookJob.job == Job.id,
-            Job.id.is_in(Job.ready_jobs))
+            Job.id.is_in(Job.ready_jobs)).order_by(Job.id)
         return (cls(job) for job in jobs)
 
 

=== modified file 'lib/lp/services/webhooks/tests/test_job.py'
--- lib/lp/services/webhooks/tests/test_job.py	2016-09-14 11:13:06 +0000
+++ lib/lp/services/webhooks/tests/test_job.py	2017-03-28 15:34:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for `WebhookJob`s."""
@@ -38,6 +38,7 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app import versioninfo
+from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.runner import JobRunner
@@ -349,6 +350,16 @@
         self.assertEqual(
             timedelta(seconds=45), removeSecurityProxy(job).soft_time_limit)
 
+    def test_iterReady_orders_by_job_id(self):
+        # Older jobs are run first.
+        hook = self.factory.makeWebhook()
+        jobs = [
+            WebhookJob(hook, WebhookJobType.DELIVERY, {}) for _ in range(3)]
+        IStore(WebhookJob).flush()
+        self.assertEqual(
+            [job.job_id for job in jobs],
+            [job.job_id for job in WebhookDeliveryJob.iterReady()])
+
     def test_run_200(self):
         # A request that returns 200 is a success.
         with CaptureOops() as oopses: