[PATCH] Only mark series as completed after cover letter has arrived
vkabatov at redhat.com
vkabatov at redhat.com
Tue Jan 15 05:12:11 AEDT 2019
From: Veronika Kabatova <vkabatov at redhat.com>
People often put metadata into the cover letter. These are needed by
various CI systems built upon Patchwork, and they are unable to process
the series if a cover letter is expected to arrive and it didn't arrive
at the time of processing. Email delaying is normal and the CI systems
are currently unable to exclude series based on the `received_all`
attribute.
Add a new DB field (not exposed in the API) that says "a cover letter
should arrive but it didn't yet" and use this field when assessing
whether the series are completed or not. In the end, a cover letter is
a vital part of the series.
Signed-off-by: Veronika Kabatova <vkabatov at redhat.com>
---
Most of the people needing this change currently have a workaround in
their pipelines.
Besides protecting automation from race conditions in email delivery,
the patch also complements the proposed tagging feature changes -- one
of the changes (issue #113) deals with adding cover letter tags to each
patch in the series. Together with the planned flattening of DB
structure around Submission/Patch/Cover letter, unifying the behavior
around cover letters and patches just makes sense.
---
.../0034_add_cover_expected_to_series.py | 22 +++++
patchwork/models.py | 23 ++++-
patchwork/signals.py | 23 ++++-
.../series/base-cover-letter-delayed.mbox | 94 +++++++++++++++++++
patchwork/tests/test_series.py | 21 ++++-
5 files changed, 178 insertions(+), 5 deletions(-)
create mode 100644 patchwork/migrations/0034_add_cover_expected_to_series.py
create mode 100644 patchwork/tests/series/base-cover-letter-delayed.mbox
diff --git a/patchwork/migrations/0034_add_cover_expected_to_series.py b/patchwork/migrations/0034_add_cover_expected_to_series.py
new file mode 100644
index 00000000..85cf7281
--- /dev/null
+++ b/patchwork/migrations/0034_add_cover_expected_to_series.py
@@ -0,0 +1,22 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+# Only add the model field. Since the cover letter should arrive within approx.
+# 10 mins form the patches (delayed emails happen), we actually don't expect to
+# wait for any old emails. They should be already there.
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('patchwork', '0033_remove_patch_series_model'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='series',
+ name='cover_expected',
+ field=models.BooleanField(default=False),
+ ),
+ ]
diff --git a/patchwork/models.py b/patchwork/models.py
index a7eee4db..d23b0d03 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -7,6 +7,7 @@
from collections import Counter
from collections import OrderedDict
import datetime
+import email.parser
import random
import re
@@ -380,6 +381,10 @@ class Submission(FilenameMixin, EmailMixin, models.Model):
class CoverLetter(Submission):
+ def delete(self, *args, **kwargs):
+ self.series.cover_expected = True
+ super(CoverLetter, self).delete(**kwargs)
+
def get_absolute_url(self):
return reverse('cover-detail', kwargs={'cover_id': self.id})
@@ -627,6 +632,7 @@ class Series(FilenameMixin, models.Model):
cover_letter = models.OneToOneField(CoverLetter, related_name='series',
null=True,
on_delete=models.CASCADE)
+ cover_expected = models.BooleanField(default=False)
# metadata
name = models.CharField(max_length=255, blank=True, null=True,
@@ -658,7 +664,7 @@ class Series(FilenameMixin, models.Model):
@property
def received_all(self):
- return self.total <= self.received_total
+ return (self.total <= self.received_total) and not self.cover_expected
def add_cover_letter(self, cover):
"""Add a cover letter to the series.
@@ -699,13 +705,24 @@ class Series(FilenameMixin, models.Model):
if self.name == name:
self.name = self._format_name(cover)
+ self.cover_expected = False
+
self.save()
def add_patch(self, patch, number):
"""Add a patch to the series."""
# both user defined names and cover letter-based names take precedence
- if not self.name and number == 1:
- self.name = patch.name # keep the prefixes for patch-based names
+ if number == 1:
+ if not self.name: # keep the prefixes for patch-based names
+ self.name = patch.name
+
+ # Find out if we expect a cover letter to arrive later
+ if not self.cover_letter:
+ in_reply_to = email.parser.Parser().parsestr(
+ patch.headers, True).get('In-Reply-To')
+ if in_reply_to:
+ self.cover_expected = True
+
self.save()
patch.series = self
diff --git a/patchwork/signals.py b/patchwork/signals.py
index 666199b6..56157ae5 100644
--- a/patchwork/signals.py
+++ b/patchwork/signals.py
@@ -211,7 +211,7 @@ def create_series_created_event(sender, instance, created, raw, **kwargs):
@receiver(pre_save, sender=Patch)
-def create_series_completed_event(sender, instance, raw, **kwargs):
+def create_series_completed_by_patch_event(sender, instance, raw, **kwargs):
# NOTE(stephenfin): It's actually possible for this event to be fired
# multiple times for a given series. To trigger this case, you would need
@@ -240,5 +240,26 @@ def create_series_completed_event(sender, instance, raw, **kwargs):
# we can't use "series.received_all" here since we haven't actually saved
# the instance yet so we duplicate that logic here but with an offset
+ if (instance.series.received_total + 1) >= instance.series.total and \
+ not instance.series.cover_expected:
+ create_event(instance.series)
+
+
+ at receiver(pre_save, sender=CoverLetter)
+def create_series_completed_by_cover_event(sender, instance, raw, **kwargs):
+
+ def create_event(series):
+ return Event.objects.create(
+ category=Event.CATEGORY_SERIES_COMPLETED,
+ project=series.project,
+ series=series)
+
+ # Don't trigger for items loaded from fixtures or new items
+ if raw or not instance.pk:
+ return
+
+ # We can't use "series.received_all" here since we haven't actually saved
+ # the instance yet so we duplicate that logic here. No need to check
+ # series.cover_expected since the cover has just arrived.
if (instance.series.received_total + 1) >= instance.series.total:
create_event(instance.series)
diff --git a/patchwork/tests/series/base-cover-letter-delayed.mbox b/patchwork/tests/series/base-cover-letter-delayed.mbox
new file mode 100644
index 00000000..e5bb7d1f
--- /dev/null
+++ b/patchwork/tests/series/base-cover-letter-delayed.mbox
@@ -0,0 +1,94 @@
+From stephenfinucane at gmail.com Sun Sep 11 23:22:13 2016
+Return-Path: <stephenfinucane at gmail.com>
+From: Stephen Finucane <stephenfinucane at gmail.com>
+To: stephenfinucane at hotmail.com
+Subject: [PATCH 1/2] test: Add some lorem ipsum
+Date: Sun, 11 Sep 2016 23:22:03 +0100
+Message-ID: <1473632524-8585-2-git-send-email-stephenfinucane at gmail.com>
+X-Mailer: git-send-email 2.7.4
+In-Reply-To: <1473632524-8585-1-git-send-email-stephenfinucane at gmail.com>
+References: <1473632524-8585-1-git-send-email-stephenfinucane at gmail.com>
+MIME-Version: 1.0
+Content-Type: text/plain
+Content-Length: 670
+Lines: 22
+
+From: Stephen Finucane <stephenfinucane at hotmail.com>
+
+---
+ test.txt | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/test.txt b/test.txt
+index f75ba05..a6c61c0 100644
+--- a/test.txt
++++ b/test.txt
+@@ -1 +1,8 @@
+ Hello, world.
++
++Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras eget eleifend
++augue. Nullam at metus venenatis, laoreet neque nec, convallis mauris.
++Pellentesque aliquam at nisi et laoreet. Duis non nisl venenatis, rhoncus risus
++id, elementum felis. In hac habitasse platea dictumst. Nam sit amet maximus
++eros. Nam quis ligula ut tortor egestas bibendum. Nunc sed purus sit amet
++tellus commodo bibendum ut vel dolor.
+--
+2.7.4
+
+
+From stephenfinucane at gmail.com Sun Sep 11 23:22:16 2016
+Return-Path: <stephenfinucane at gmail.com>
+From: Stephen Finucane <stephenfinucane at gmail.com>
+To: stephenfinucane at hotmail.com
+Subject: [PATCH 2/2] test: Convert to Markdown
+Date: Sun, 11 Sep 2016 23:22:04 +0100
+Message-ID: <1473632524-8585-3-git-send-email-stephenfinucane at gmail.com>
+X-Mailer: git-send-email 2.7.4
+In-Reply-To: <1473632524-8585-1-git-send-email-stephenfinucane at gmail.com>
+References: <1473632524-8585-1-git-send-email-stephenfinucane at gmail.com>
+MIME-Version: 1.0
+Content-Type: text/plain
+Content-Length: 1345
+Lines: 40
+
+From: Stephen Finucane <stephenfinucane at hotmail.com>
+
+---
+ test.md | 8 ++++++++
+ test.txt | 8 --------
+ 2 files changed, 8 insertions(+), 8 deletions(-)
+ create mode 100644 test.md
+ delete mode 100644 test.txt
+
+diff --git a/test.md b/test.md
+new file mode 100644
+index 0000000..e5ff90e
+--- /dev/null
++++ b/test.md
+@@ -0,0 +1,8 @@
++# Hello, world
++
++Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras eget eleifend
++augue. Nullam at metus venenatis, laoreet neque nec, convallis mauris.
++Pellentesque aliquam at nisi et laoreet. Duis non nisl venenatis, rhoncus risus
++id, elementum felis. In hac habitasse platea dictumst. Nam sit amet maximus
++eros. Nam quis ligula ut tortor egestas bibendum. Nunc sed purus sit amet
++tellus commodo bibendum ut vel dolor.
+diff --git a/test.txt b/test.txt
+deleted file mode 100644
+index a6c61c0..0000000
+--- a/test.txt
++++ /dev/null
+@@ -1,8 +0,0 @@
+-Hello, world.
+-
+-Lorem ipsum dolor sit amet, consectetur adipiscing elit. Cras eget eleifend
+-augue. Nullam at metus venenatis, laoreet neque nec, convallis mauris.
+-Pellentesque aliquam at nisi et laoreet. Duis non nisl venenatis, rhoncus risus
+-id, elementum felis. In hac habitasse platea dictumst. Nam sit amet maximus
+-eros. Nam quis ligula ut tortor egestas bibendum. Nunc sed purus sit amet
+-tellus commodo bibendum ut vel dolor.
+--
+2.7.4
+
+
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py
index f5e6b5b0..8eda806b 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -531,7 +531,8 @@ class SeriesTotalTest(_BaseTestCase):
def test_complete(self):
"""Series received with expected number of patches.
- Parse a series where all patches are received as expected.
+ Parse a series where all patches and a cover letter are received as
+ expected.
Input:
@@ -548,6 +549,24 @@ class SeriesTotalTest(_BaseTestCase):
series = patches[0].series
self.assertTrue(series.received_all)
+ def test_cover_delayed(self):
+ """
+ Check that series are mark as not complete if a cover is expected but
+ didn't arrive yet.
+
+ Input:
+
+ - [PATCH 1/2] test: Add some lorem ipsum
+ - [PATCH 2/2] test: Convert to Markdown
+ """
+ covers, patches, _ = self._parse_mbox(
+ 'base-cover-letter-delayed.mbox', [0, 2, 0])
+
+ self.assertSerialized(patches, [2])
+
+ series = patches[0].series
+ self.assertFalse(series.received_all)
+
def test_extra_patches(self):
"""Series received with additional patches.
--
2.17.2
More information about the Patchwork
mailing list