launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/no-logtail-only-webhooks into lp:launchpad.

Commit message:
Only emit snap:build:0.1 webhooks from SnapBuild.updateStatus if the status has changed.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/no-logtail-only-webhooks/+merge/336479
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/no-logtail-only-webhooks into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2017-04-27 16:22:37 +0000
+++ lib/lp/snappy/model/snapbuild.py	2018-01-23 13:39:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -335,6 +335,7 @@
                      date_started=None, date_finished=None,
                      force_invalid_transition=False):
         """See `IBuildFarmJob`."""
+        old_status = self.status
         super(SnapBuild, self).updateStatus(
             status, builder=builder, slave_status=slave_status,
             date_started=date_started, date_finished=date_finished,
@@ -343,7 +344,8 @@
             revision_id = slave_status.get("revision_id")
             if revision_id is not None:
                 self.revision_id = unicode(revision_id)
-        notify(SnapBuildStatusChangedEvent(self))
+        if status != old_status:
+            notify(SnapBuildStatusChangedEvent(self))
 
     def notify(self, extra_info=None):
         """See `IPackageBuild`."""

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2017-10-20 13:35:42 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2018-01-23 13:39:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap package build features."""
@@ -255,6 +255,22 @@
                     hook.id, hook.target),
                 repr(delivery))
 
+    def test_updateStatus_no_change_does_not_trigger_webhooks(self):
+        # An updateStatus call that changes details such as the revision_id
+        # but that doesn't change the build's status attribute does not
+        # trigger webhooks.
+        hook = self.factory.makeWebhook(
+            target=self.build.snap, event_types=["snap:build:0.1"])
+        builder = self.factory.makeBuilder()
+        self.build.updateStatus(BuildStatus.BUILDING)
+        self.assertEqual(1, hook.deliveries.count())
+        self.build.updateStatus(
+            BuildStatus.BUILDING, builder=builder,
+            slave_status={"revision_id": "1"})
+        self.assertEqual(1, hook.deliveries.count())
+        self.build.updateStatus(BuildStatus.UPLOADING)
+        self.assertEqual(2, hook.deliveries.count())
+
     def test_updateStatus_failure_does_not_trigger_store_uploads(self):
         # A failed SnapBuild does not trigger store uploads.
         self.build.snap.store_series = self.factory.makeSnappySeries()