Launchpad itself

Merge lp:~cjwatson/launchpad/webhook-job-ordering into lp:launchpad

  1. webhook-job-ordering
  2. Merge into devel
Proposed by Colin Watson
Status: Merged
Merged at revision: 18337
Proposed branch: lp:~cjwatson/launchpad/webhook-job-ordering
Merge into: lp:launchpad
Diff against target: 53 lines (+14/-3)
2 files modified
lib/lp/services/webhooks/model.py (+2/-2)
lib/lp/services/webhooks/tests/test_job.py (+12/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/webhook-job-ordering
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+321188@code.launchpad.net

Commit message

Order webhook delivery jobs by job ID.

Description of the change

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.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/webhooks/model.py'
2--- lib/lp/services/webhooks/model.py 2016-09-14 11:13:06 +0000
3+++ lib/lp/services/webhooks/model.py 2017-03-28 15:34:29 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
6+# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 __metaclass__ = type
10@@ -366,7 +366,7 @@
11 WebhookJob,
12 WebhookJob.job_type == cls.class_job_type,
13 WebhookJob.job == Job.id,
14- Job.id.is_in(Job.ready_jobs))
15+ Job.id.is_in(Job.ready_jobs)).order_by(Job.id)
16 return (cls(job) for job in jobs)
17
18
19
20=== modified file 'lib/lp/services/webhooks/tests/test_job.py'
21--- lib/lp/services/webhooks/tests/test_job.py 2016-09-14 11:13:06 +0000
22+++ lib/lp/services/webhooks/tests/test_job.py 2017-03-28 15:34:29 +0000
23@@ -1,4 +1,4 @@
24-# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
25+# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
26 # GNU Affero General Public License version 3 (see the file LICENSE).
27
28 """Tests for `WebhookJob`s."""
29@@ -38,6 +38,7 @@
30 from zope.security.proxy import removeSecurityProxy
31
32 from lp.app import versioninfo
33+from lp.services.database.interfaces import IStore
34 from lp.services.features.testing import FeatureFixture
35 from lp.services.job.interfaces.job import JobStatus
36 from lp.services.job.runner import JobRunner
37@@ -349,6 +350,16 @@
38 self.assertEqual(
39 timedelta(seconds=45), removeSecurityProxy(job).soft_time_limit)
40
41+ def test_iterReady_orders_by_job_id(self):
42+ # Older jobs are run first.
43+ hook = self.factory.makeWebhook()
44+ jobs = [
45+ WebhookJob(hook, WebhookJobType.DELIVERY, {}) for _ in range(3)]
46+ IStore(WebhookJob).flush()
47+ self.assertEqual(
48+ [job.job_id for job in jobs],
49+ [job.job_id for job in WebhookDeliveryJob.iterReady()])
50+
51 def test_run_200(self):
52 # A request that returns 200 is a success.
53 with CaptureOops() as oopses: