Flip model, for tracking status changes of the Check objects.

CHANGELOG.md

@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
 - Additional python usage examples
 - Allow simultaneous access to checks from different teams
 - Add CORS support to API endpoints
+- Flip model, for tracking status changes of the Check objects
 
 ### Bug Fixes
 - Fix after-login redirects (the "?next=" query parameter)

hc/api/admin.py

@@ -2,7 +2,7 @@ from django.contrib import admin
 from django.core.paginator import Paginator
 from django.db import connection
 from django.utils.safestring import mark_safe
-from hc.api.models import Channel, Check, Notification, Ping
+from hc.api.models import Channel, Check, Flip, Notification, Ping
 from hc.lib.date import format_duration
 
 
@@ -197,3 +197,8 @@ class NotificationsAdmin(admin.ModelAdmin):
 
     def channel_value(self, obj):
         return obj.channel.value
+
+
+@admin.register(Flip)
+class FlipsAdmin(admin.ModelAdmin):
+    list_display = ("id", "created", "owner", "old_status", "new_status")

hc/api/management/commands/sendalerts.py

@@ -3,32 +3,39 @@ from threading import Thread
 
 from django.core.management.base import BaseCommand
 from django.utils import timezone
-from hc.api.models import Check
+from hc.api.models import Check, Flip
 
 
-def notify(check_id, stdout):
+def notify(flip_id, stdout):
-    check = Check.objects.get(id=check_id)
+    flip = Flip.objects.get(id=flip_id)
+
+    check = flip.owner
+    # Set the historic status here but *don't save it*.
+    # It would be nicer to pass the status explicitly, as a separate parameter.
+    check.status = flip.new_status
+    # And just to make sure it doesn't get saved by a future coding accident:
+    setattr(check, "save", None)
+
     tmpl = "Sending alert, status=%s, code=%s\n"
-    stdout.write(tmpl % (check.status, check.code))
+    stdout.write(tmpl % (flip.new_status, check.code))
 
     # Set dates for followup nags
-    if check.status == "down" and check.user.profile:
+    if flip.new_status == "down" and check.user.profile:
         check.user.profile.set_next_nag_date()
 
     # Send notifications
-    errors = check.send_alert()
+    errors = check.send_alert(flip)
     for ch, error in errors:
         stdout.write("ERROR: %s %s %s\n" % (ch.kind, ch.value, error))
 
 
-def notify_on_thread(check_id, stdout):
+def notify_on_thread(flip_id, stdout):
-    t = Thread(target=notify, args=(check_id, stdout))
+    t = Thread(target=notify, args=(flip_id, stdout))
     t.start()
 
 
 class Command(BaseCommand):
     help = 'Sends UP/DOWN email alerts'
-    owned = Check.objects.filter(user__isnull=False).order_by("alert_after")
 
     def add_arguments(self, parser):
         parser.add_argument(
@@ -47,56 +54,73 @@ class Command(BaseCommand):
             help='Send alerts synchronously, without using threads',
         )
 
-    def handle_one(self, use_threads=True):
+    def process_one_flip(self, use_threads=True):
-        """ Process a single check.  """
+        """ Find unprocessed flip, send notifications.  """
+
+        # Order by processed, otherwise Django will automatically order by id
+        # and make the query less efficient
+        q = Flip.objects.filter(processed=None).order_by("processed")
+        flip = q.first()
+        if flip is None:
+            return False
+
+        q = Flip.objects.filter(id=flip.id, processed=None)
+        num_updated = q.update(processed=timezone.now())
+        if num_updated != 1:
+            # Nothing got updated: another worker process got there first.
+            return True
+
+        if use_threads:
+            notify_on_thread(flip.id, self.stdout)
+        else:
+            notify(flip.id, self.stdout)
+
+        return True
+
+    def handle_going_down(self):
+        """ Process a single check going down.  """
 
         now = timezone.now()
 
-        # Look for checks that are going down
+        check = Check.objects.filter(alert_after__lt=now, status="up").first()
-        q = self.owned.filter(alert_after__lt=now, status="up")
-        check = q.first()
-
-        # If none found, look for checks that are going up
-        if not check:
-            q = self.owned.filter(alert_after__gt=now, status="down")
-            check = q.first()
-
         if check is None:
             return False
 
-        q = Check.objects.filter(id=check.id, status=check.status)
+        q = Check.objects.filter(id=check.id, status="up")
+
         current_status = check.get_status()
-
+        if current_status != "down":
-        # During the grace period sendalerts considers the check as "up":
+            # It is not down yet. Update alert_after
-        if current_status == "grace":
-            current_status = "up"
-
-        if check.status == current_status:
-            # Stored status is already up-to-date. Update alert_after
-            # as needed but don't send notifications
             q.update(alert_after=check.get_alert_after())
             return True
-        else:
-            # Atomically update status to the opposite
-            num_updated = q.update(status=current_status)
-            if num_updated == 1:
-                # Send notifications only if status update succeeded
-                # (no other sendalerts process got there first)
-                if use_threads:
-                    notify_on_thread(check.id, self.stdout)
-                else:
-                    notify(check.id, self.stdout)
 
-                return True
+        # Atomically update status
+        num_updated = q.update(status="down")
+        if num_updated != 1:
+            # Nothing got updated: another worker process got there first.
+            return True
 
-        return False
+        flip = Flip(owner=check)
+        flip.created = check.get_alert_after()
+        flip.old_status = "up"
+        flip.new_status = "down"
+        flip.save()
+
+        check.status = "down"
+        check.save()
+        return True
 
     def handle(self, use_threads=True, loop=True, *args, **options):
         self.stdout.write("sendalerts is now running\n")
 
         i, sent = 0, 0
         while True:
-            while self.handle_one(use_threads):
+            # Create flips for any checks going down
+            while self.handle_going_down():
+                pass
+
+            # Process the unprocessed flips
+            while self.process_one_flip(use_threads):
                 sent += 1
 
             if not loop:

hc/api/migrations/0045_flip.py

@@ -0,0 +1,25 @@
+# Generated by Django 2.1.4 on 2018-12-10 15:15
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0044_auto_20181120_2004'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Flip',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('created', models.DateTimeField()),
+                ('processed', models.DateTimeField(blank=True, db_index=True, null=True)),
+                ('old_status', models.CharField(choices=[('up', 'Up'), ('down', 'Down'), ('new', 'New'), ('paused', 'Paused')], max_length=8)),
+                ('new_status', models.CharField(choices=[('up', 'Up'), ('down', 'Down'), ('new', 'New'), ('paused', 'Paused')], max_length=8)),
+                ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='api.Check')),
+            ],
+        ),
+    ]

hc/api/models.py

@@ -92,8 +92,12 @@ class Check(models.Model):
     def email(self):
         return "%s@%s" % (self.code, settings.PING_EMAIL_DOMAIN)
 
-    def send_alert(self):
+    def send_alert(self, flip):
-        if self.status not in ("up", "down"):
+        if flip.new_status == "up" and flip.old_status in ("new", "paused"):
+            # Don't send alerts on new->up and paused->up transitions
+            return []
+
+        if flip.new_status not in ("up", "down"):
             raise NotImplementedError("Unexpected status: %s" % self.status)
 
         errors = []
@@ -199,8 +203,16 @@ class Check(models.Model):
         self.last_ping_was_fail = is_fail
         self.has_confirmation_link = "confirm" in str(body).lower()
         self.alert_after = self.get_alert_after()
-        if self.status in ("new", "paused"):
+
-            self.status = "up"
+        new_status = "down" if is_fail else "up"
+        if self.status != new_status:
+            flip = Flip(owner=self)
+            flip.created = self.last_ping
+            flip.old_status = self.status
+            flip.new_status = new_status
+            flip.save()
+
+            self.status = new_status
 
         self.save()
         self.refresh_from_db()
@@ -539,3 +551,11 @@ class Notification(models.Model):
 
     def bounce_url(self):
         return settings.SITE_ROOT + reverse("hc-api-bounce", args=[self.code])
+
+
+class Flip(models.Model):
+    owner = models.ForeignKey(Check, models.CASCADE)
+    created = models.DateTimeField()
+    processed = models.DateTimeField(null=True, blank=True, db_index=True)
+    old_status = models.CharField(max_length=8, choices=STATUSES)
+    new_status = models.CharField(max_length=8, choices=STATUSES)

hc/api/tests/test_sendalerts.py

@@ -5,7 +5,7 @@ from mock import Mock, patch
 from django.core.management import call_command
 from django.utils.timezone import now
 from hc.api.management.commands.sendalerts import Command, notify
-from hc.api.models import Check
+from hc.api.models import Flip, Check
 from hc.test import BaseTestCase
 
 
@@ -18,50 +18,54 @@ class SendAlertsTestCase(BaseTestCase):
         check.alert_after = check.get_alert_after()
         check.save()
 
-        # Expect no exceptions--
+        Command().handle_going_down()
-        Command().handle_one()
 
-    @patch("hc.api.management.commands.sendalerts.notify_on_thread")
+        self.assertEqual(Flip.objects.count(), 0)
-    def test_it_notifies_when_check_goes_down(self, mock_notify):
+
+    def test_it_creates_a_flip_when_check_goes_down(self):
         check = Check(user=self.alice, status="up")
         check.last_ping = now() - timedelta(days=2)
         check.alert_after = check.get_alert_after()
         check.save()
 
-        result = Command().handle_one()
+        result = Command().handle_going_down()
 
         # If it finds work, it should return True
         self.assertTrue(result)
 
+        # It should create a flip object
+        flip = Flip.objects.get()
+        self.assertEqual(flip.owner_id, check.id)
+        self.assertEqual(flip.new_status, "down")
+
         # It should change stored status to "down"
         check.refresh_from_db()
         self.assertEqual(check.status, "down")
 
-        # It should call `notify_on_thread`
-        self.assertTrue(mock_notify.called)
-
     @patch("hc.api.management.commands.sendalerts.notify_on_thread")
-    def test_it_notifies_when_check_goes_up(self, mock_notify):
+    def test_it_processes_flip(self, mock_notify):
-        check = Check(user=self.alice, status="down")
+        check = Check(user=self.alice, status="up")
         check.last_ping = now()
         check.alert_after = check.get_alert_after()
         check.save()
 
-        result = Command().handle_one()
+        flip = Flip(owner=check, created=check.last_ping)
+        flip.old_status = "down"
+        flip.new_status = "up"
+        flip.save()
+
+        result = Command().process_one_flip()
 
         # If it finds work, it should return True
         self.assertTrue(result)
 
-        # It should change stored status to "up"
+        # It should set the processed date
-        check.refresh_from_db()
+        flip.refresh_from_db()
-        self.assertEqual(check.status, "up")
+        self.assertTrue(flip.processed)
 
         # It should call `notify_on_thread`
         self.assertTrue(mock_notify.called)
 
-        # alert_after now should be set
-        self.assertTrue(check.alert_after)
-
     @patch("hc.api.management.commands.sendalerts.notify_on_thread")
     def test_it_updates_alert_after(self, mock_notify):
         check = Check(user=self.alice, status="up")
@@ -69,19 +73,17 @@ class SendAlertsTestCase(BaseTestCase):
         check.alert_after = check.last_ping
         check.save()
 
-        result = Command().handle_one()
+        result = Command().handle_going_down()
 
         # If it finds work, it should return True
         self.assertTrue(result)
 
-        # It should change stored status to "down"
-        check.refresh_from_db()
-
         # alert_after should have been increased
+        check.refresh_from_db()
         self.assertTrue(check.alert_after > check.last_ping)
 
-        # notify_on_thread should *not* have been called
+        # a flip should have not been created
-        self.assertFalse(mock_notify.called)
+        self.assertEqual(Flip.objects.count(), 0)
 
     @patch("hc.api.management.commands.sendalerts.notify")
     def test_it_works_synchronously(self, mock_notify):
@@ -105,7 +107,12 @@ class SendAlertsTestCase(BaseTestCase):
         check.alert_after = check.get_alert_after()
         check.save()
 
-        notify(check.id, Mock())
+        flip = Flip(owner=check, created=check.last_ping)
+        flip.old_status = "up"
+        flip.new_status = "down"
+        flip.save()
+
+        notify(flip.id, Mock())
 
         self.profile.refresh_from_db()
         self.assertIsNotNone(self.profile.next_nag_date)
@@ -119,7 +126,12 @@ class SendAlertsTestCase(BaseTestCase):
         check.alert_after = check.get_alert_after()
         check.save()
 
-        notify(check.id, Mock())
+        flip = Flip(owner=check, created=check.last_ping)
+        flip.old_status = "up"
+        flip.new_status = "down"
+        flip.save()
+
+        notify(flip.id, Mock())
 
         self.bobs_profile.refresh_from_db()
         self.assertIsNotNone(self.bobs_profile.next_nag_date)
@@ -135,7 +147,12 @@ class SendAlertsTestCase(BaseTestCase):
         check.alert_after = check.get_alert_after()
         check.save()
 
-        notify(check.id, Mock())
+        flip = Flip(owner=check, created=check.last_ping)
+        flip.old_status = "up"
+        flip.new_status = "down"
+        flip.save()
+
+        notify(flip.id, Mock())
 
         self.profile.refresh_from_db()
-        self.assertEqual(self.profile.next_nag_date, original_nag_date)
+        self.assertEqual(self.profile.next_nag_date, original_nag_date)