diff --git a/luigi/batch_notifier.py b/luigi/batch_notifier.py index d7a1ffd01c..d046ee3a91 100644 --- a/luigi/batch_notifier.py +++ b/luigi/batch_notifier.py @@ -187,19 +187,21 @@ def _send_email(self, fail_counts, disable_counts, scheduling_counts, fail_expls send_email(subject, email_body, email().sender, (owner,)) def send_email(self): - for owner, failures in six.iteritems(self._fail_counts): - self._send_email( - fail_counts=failures, - disable_counts=self._disabled_counts[owner], - scheduling_counts=self._scheduling_fail_counts[owner], - fail_expls=self._fail_expls[owner], - owner=owner, - ) - self._update_next_send() - self._fail_counts.clear() - self._disabled_counts.clear() - self._scheduling_fail_counts.clear() - self._fail_expls.clear() + try: + for owner, failures in six.iteritems(self._fail_counts): + self._send_email( + fail_counts=failures, + disable_counts=self._disabled_counts[owner], + scheduling_counts=self._scheduling_fail_counts[owner], + fail_expls=self._fail_expls[owner], + owner=owner, + ) + finally: + self._update_next_send() + self._fail_counts.clear() + self._disabled_counts.clear() + self._scheduling_fail_counts.clear() + self._fail_expls.clear() def update(self): if time.time() >= self._next_send: diff --git a/test/batch_notifier_test.py b/test/batch_notifier_test.py index 804cbcd875..a31f527bab 100644 --- a/test/batch_notifier_test.py +++ b/test/batch_notifier_test.py @@ -1,4 +1,5 @@ # coding=utf-8 +from smtplib import SMTPServerDisconnected import mock import unittest @@ -292,6 +293,17 @@ def test_send_clears_backlog(self): bn.send_email() self.send_email.assert_not_called() + def test_email_gets_cleared_on_failure(self): + bn = BatchNotifier(batch_mode='all') + + bn.add_failure('Task(a=5)', 'Task', {'a': 1}, '', []) + self.send_email.side_effect = SMTPServerDisconnected('timeout') + self.assertRaises(SMTPServerDisconnected, bn.send_email) + + self.send_email.reset_mock() + bn.send_email() + self.send_email.assert_not_called() + def test_send_clears_all_old_data(self): bn = BatchNotifier(batch_mode='all', error_messages=100) @@ -362,6 +374,25 @@ def test_no_auto_send_until_end_of_interval_with_error(self): '- Task(a=5) (1 failure)' ) + def test_no_auto_send_for_interval_after_exception(self): + bn = BatchNotifier(batch_mode='all') + bn.add_failure('Task(a=5)', 'Task', {'a': 5}, 'error', []) + self.send_email.side_effect = SMTPServerDisconnected + + self.incr_time(minutes=60) + self.assertRaises(SMTPServerDisconnected, bn.update) + + self.send_email.reset_mock() + self.send_email.side_effect = None + bn.add_failure('Task(a=5)', 'Task', {'a': 5}, 'error', []) + for i in range(60): + bn.update() + self.send_email.assert_not_called() + self.incr_time(minutes=1) + + bn.update() + self.assertEqual(1, self.send_email.call_count) + def test_send_batch_failure_emails_to_owners(self): bn = BatchNotifier(batch_mode='all') bn.add_failure('Task(a=1)', 'Task', {'a': '1'}, 'error', ['a@test.com', 'b@test.com'])