Commit 08dfe5e9 by Giovanni Di Milia

Fixed pylint violation for files in lms/djangoapps/bulk_email

parent 2b5810db
...@@ -54,7 +54,9 @@ unsupported tags will cause email sending to fail. ...@@ -54,7 +54,9 @@ unsupported tags will cause email sending to fail.
return True return True
def has_delete_permission(self, request, obj=None): def has_delete_permission(self, request, obj=None):
"""Disables the ability to remove existing templates, as we'd like to make sure we don't have dangling references.""" """
Disables the ability to remove existing templates, as we'd like to make sure we don't have dangling references.
"""
return False return False
......
...@@ -75,7 +75,9 @@ class CourseEmail(Email): ...@@ -75,7 +75,9 @@ class CourseEmail(Email):
return self.subject return self.subject
@classmethod @classmethod
def create(cls, course_id, sender, to_option, subject, html_message, text_message=None, template_name=None, from_addr=None): def create(
cls, course_id, sender, to_option, subject, html_message,
text_message=None, template_name=None, from_addr=None):
""" """
Create an instance of CourseEmail. Create an instance of CourseEmail.
......
...@@ -25,9 +25,9 @@ from boto.ses.exceptions import ( ...@@ -25,9 +25,9 @@ from boto.ses.exceptions import (
) )
from boto.exception import AWSConnectionError from boto.exception import AWSConnectionError
from celery import task, current_task from celery import task, current_task # pylint: disable=no-name-in-module
from celery.states import SUCCESS, FAILURE, RETRY from celery.states import SUCCESS, FAILURE, RETRY # pylint: disable=no-name-in-module, import-error
from celery.exceptions import RetryTaskError from celery.exceptions import RetryTaskError # pylint: disable=no-name-in-module, import-error
from django.conf import settings from django.conf import settings
from django.contrib.auth.models import User from django.contrib.auth.models import User
...@@ -310,7 +310,8 @@ def send_course_email(entry_id, email_id, to_list, global_email_context, subtask ...@@ -310,7 +310,8 @@ def send_course_email(entry_id, email_id, to_list, global_email_context, subtask
subtask_status = SubtaskStatus.from_dict(subtask_status_dict) subtask_status = SubtaskStatus.from_dict(subtask_status_dict)
current_task_id = subtask_status.task_id current_task_id = subtask_status.task_id
num_to_send = len(to_list) num_to_send = len(to_list)
log.info(u"Preparing to send email %s to %d recipients as subtask %s for instructor task %d: context = %s, status=%s", log.info((u"Preparing to send email %s to %d recipients as subtask %s "
u"for instructor task %d: context = %s, status=%s"),
email_id, num_to_send, current_task_id, entry_id, global_email_context, subtask_status) email_id, num_to_send, current_task_id, entry_id, global_email_context, subtask_status)
# Check that the requested subtask is actually known to the current InstructorTask entry. # Check that the requested subtask is actually known to the current InstructorTask entry.
...@@ -663,20 +664,22 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas ...@@ -663,20 +664,22 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
except BULK_EMAIL_FAILURE_ERRORS as exc: except BULK_EMAIL_FAILURE_ERRORS as exc:
dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)]) dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)])
num_pending = len(to_list) num_pending = len(to_list)
log.exception('Task %s: email with id %d caused send_course_email task to fail with "fatal" exception. %d emails unsent.', log.exception(('Task %s: email with id %d caused send_course_email task to fail '
'with "fatal" exception. %d emails unsent.'),
task_id, email_id, num_pending) task_id, email_id, num_pending)
# Update counters with progress to date, counting unsent emails as failures, # Update counters with progress to date, counting unsent emails as failures,
# and set the state to FAILURE: # and set the state to FAILURE:
subtask_status.increment(failed=num_pending, state=FAILURE) subtask_status.increment(failed=num_pending, state=FAILURE)
return subtask_status, exc return subtask_status, exc
except Exception as exc: except Exception as exc: # pylint: disable=broad-except
# Errors caught here cause the email to be retried. The entire task is actually retried # Errors caught here cause the email to be retried. The entire task is actually retried
# without popping the current recipient off of the existing list. # without popping the current recipient off of the existing list.
# These are unexpected errors. Since they might be due to a temporary condition that might # These are unexpected errors. Since they might be due to a temporary condition that might
# succeed on retry, we give them a retry. # succeed on retry, we give them a retry.
dog_stats_api.increment('course_email.limited_retry', tags=[_statsd_tag(course_title)]) dog_stats_api.increment('course_email.limited_retry', tags=[_statsd_tag(course_title)])
log.exception('Task %s: email with id %d caused send_course_email task to fail with unexpected exception. Generating retry.', log.exception(('Task %s: email with id %d caused send_course_email task to fail '
'with unexpected exception. Generating retry.'),
task_id, email_id) task_id, email_id)
# Increment the "retried_withmax" counter, update other counters with progress to date, # Increment the "retried_withmax" counter, update other counters with progress to date,
# and set the state to RETRY: # and set the state to RETRY:
...@@ -709,7 +712,8 @@ def _get_current_task(): ...@@ -709,7 +712,8 @@ def _get_current_task():
return current_task return current_task
def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current_exception, subtask_status, skip_retry_max=False): def _submit_for_retry(entry_id, email_id, to_list, global_email_context,
current_exception, subtask_status, skip_retry_max=False):
""" """
Helper function to requeue a task for retry, using the new version of arguments provided. Helper function to requeue a task for retry, using the new version of arguments provided.
...@@ -762,7 +766,8 @@ def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current ...@@ -762,7 +766,8 @@ def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current
# retries are deferred by the same amount. # retries are deferred by the same amount.
countdown = ((2 ** retry_index) * base_delay) * random.uniform(.75, 1.25) countdown = ((2 ** retry_index) * base_delay) * random.uniform(.75, 1.25)
log.warning('Task %s: email with id %d not delivered due to %s error %s, retrying send to %d recipients in %s seconds (with max_retry=%s)', log.warning(('Task %s: email with id %d not delivered due to %s error %s, '
'retrying send to %d recipients in %s seconds (with max_retry=%s)'),
task_id, email_id, exception_type, current_exception, len(to_list), countdown, max_retries) task_id, email_id, exception_type, current_exception, len(to_list), countdown, max_retries)
# we make sure that we update the InstructorTask with the current subtask status # we make sure that we update the InstructorTask with the current subtask status
...@@ -793,7 +798,7 @@ def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current ...@@ -793,7 +798,7 @@ def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current
log.exception(u'Task %s: email with id %d caused send_course_email task to retry.', log.exception(u'Task %s: email with id %d caused send_course_email task to retry.',
task_id, email_id) task_id, email_id)
return subtask_status, retry_error return subtask_status, retry_error
except Exception as retry_exc: except Exception as retry_exc: # pylint: disable=broad-except
# If there are no more retries, because the maximum has been reached, # If there are no more retries, because the maximum has been reached,
# we expect the original exception to be raised. We catch it here # we expect the original exception to be raised. We catch it here
# (and put it in retry_exc just in case it's different, but it shouldn't be), # (and put it in retry_exc just in case it's different, but it shouldn't be),
......
...@@ -4,7 +4,7 @@ Unit tests for handling email sending errors ...@@ -4,7 +4,7 @@ Unit tests for handling email sending errors
""" """
from itertools import cycle from itertools import cycle
from celery.states import SUCCESS, RETRY from celery.states import SUCCESS, RETRY # pylint: disable=no-name-in-module, import-error
from django.conf import settings from django.conf import settings
from django.core.management import call_command from django.core.management import call_command
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
......
...@@ -60,7 +60,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase): ...@@ -60,7 +60,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
"Course authorization with this Course id already exists.", "Course authorization with this Course id already exists.",
form._errors['course_id'][0] # pylint: disable=protected-access form._errors['course_id'][0] # pylint: disable=protected-access
) )
with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): with self.assertRaisesRegexp(
ValueError,
"The CourseAuthorization could not be created because the data didn't validate."
):
form.save() form.save()
# Course should still be authorized (invalid attempt had no effect) # Course should still be authorized (invalid attempt had no effect)
...@@ -81,7 +84,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase): ...@@ -81,7 +84,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
msg += 'Please recheck that you have supplied a valid course id.' msg += 'Please recheck that you have supplied a valid course id.'
self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access
with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): with self.assertRaisesRegexp(
ValueError,
"The CourseAuthorization could not be created because the data didn't validate."
):
form.save() form.save()
@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
...@@ -96,7 +102,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase): ...@@ -96,7 +102,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
msg += 'Please recheck that you have supplied a valid course id.' msg += 'Please recheck that you have supplied a valid course id.'
self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access
with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): with self.assertRaisesRegexp(
ValueError,
"The CourseAuthorization could not be created because the data didn't validate."
):
form.save() form.save()
@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
...@@ -107,11 +116,14 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase): ...@@ -107,11 +116,14 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
# Validation shouldn't work # Validation shouldn't work
self.assertFalse(form.is_valid()) self.assertFalse(form.is_valid())
error_msg = form._errors['course_id'][0] error_msg = form._errors['course_id'][0] # pylint: disable=protected-access
self.assertIn(u'--- Entered course id was: "{0}". '.format(self.course.id.run), error_msg) self.assertIn(u'--- Entered course id was: "{0}". '.format(self.course.id.run), error_msg)
self.assertIn(u'Please recheck that you have supplied a valid course id.', error_msg) self.assertIn(u'Please recheck that you have supplied a valid course id.', error_msg)
with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): with self.assertRaisesRegexp(
ValueError,
"The CourseAuthorization could not be created because the data didn't validate."
):
form.save() form.save()
...@@ -134,7 +146,10 @@ class CourseAuthorizationXMLFormTest(ModuleStoreTestCase): ...@@ -134,7 +146,10 @@ class CourseAuthorizationXMLFormTest(ModuleStoreTestCase):
msg += u'"{0}" appears to be an XML backed course.'.format(course_id.to_deprecated_string()) msg += u'"{0}" appears to be an XML backed course.'.format(course_id.to_deprecated_string())
self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access
with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."): with self.assertRaisesRegexp(
ValueError,
"The CourseAuthorization could not be created because the data didn't validate."
):
form.save() form.save()
......
...@@ -40,7 +40,9 @@ class CourseEmailTest(TestCase): ...@@ -40,7 +40,9 @@ class CourseEmailTest(TestCase):
html_message = "<html>dummy message</html>" html_message = "<html>dummy message</html>"
template_name = "branded_template" template_name = "branded_template"
from_addr = "branded@branding.com" from_addr = "branded@branding.com"
email = CourseEmail.create(course_id, sender, to_option, subject, html_message, template_name=template_name, from_addr=from_addr) email = CourseEmail.create(
course_id, sender, to_option, subject, html_message, template_name=template_name, from_addr=from_addr
)
self.assertEquals(email.course_id, course_id) self.assertEquals(email.course_id, course_id)
self.assertEquals(email.to_option, SEND_TO_STAFF) self.assertEquals(email.to_option, SEND_TO_STAFF)
self.assertEquals(email.subject, subject) self.assertEquals(email.subject, subject)
......
...@@ -25,7 +25,7 @@ from boto.ses.exceptions import ( ...@@ -25,7 +25,7 @@ from boto.ses.exceptions import (
) )
from boto.exception import AWSConnectionError from boto.exception import AWSConnectionError
from celery.states import SUCCESS, FAILURE from celery.states import SUCCESS, FAILURE # pylint: disable=no-name-in-module, import-error
from django.conf import settings from django.conf import settings
from django.core.management import call_command from django.core.management import call_command
...@@ -96,7 +96,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): ...@@ -96,7 +96,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
""" """
to_option = SEND_TO_ALL to_option = SEND_TO_ALL
course_id = course_id or self.course.id course_id = course_id or self.course.id
course_email = CourseEmail.create(course_id, self.instructor, to_option, "Test Subject", "<p>This is a test message</p>") course_email = CourseEmail.create(
course_id, self.instructor, to_option, "Test Subject", "<p>This is a test message</p>"
)
task_input = {'email_id': course_email.id} # pylint: disable=no-member task_input = {'email_id': course_email.id} # pylint: disable=no-member
task_id = str(uuid4()) task_id = str(uuid4())
instructor_task = InstructorTaskFactory.create( instructor_task = InstructorTaskFactory.create(
...@@ -153,7 +155,7 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): ...@@ -153,7 +155,7 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
self.assertEquals(len(task_id_list), 1) self.assertEquals(len(task_id_list), 1)
task_id = task_id_list[0] task_id = task_id_list[0]
subtask_status = subtask_status_info.get(task_id) subtask_status = subtask_status_info.get(task_id)
print("Testing subtask status: {}".format(subtask_status)) print "Testing subtask status: {}".format(subtask_status)
self.assertEquals(subtask_status.get('task_id'), task_id) self.assertEquals(subtask_status.get('task_id'), task_id)
self.assertEquals(subtask_status.get('attempted'), succeeded + failed) self.assertEquals(subtask_status.get('attempted'), succeeded + failed)
self.assertEquals(subtask_status.get('succeeded'), succeeded) self.assertEquals(subtask_status.get('succeeded'), succeeded)
...@@ -163,7 +165,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): ...@@ -163,7 +165,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
self.assertEquals(subtask_status.get('retried_withmax'), retried_withmax) self.assertEquals(subtask_status.get('retried_withmax'), retried_withmax)
self.assertEquals(subtask_status.get('state'), SUCCESS if succeeded > 0 else FAILURE) self.assertEquals(subtask_status.get('state'), SUCCESS if succeeded > 0 else FAILURE)
def _test_run_with_task(self, task_class, action_name, total, succeeded, failed=0, skipped=0, retried_nomax=0, retried_withmax=0): def _test_run_with_task(
self, task_class, action_name, total, succeeded,
failed=0, skipped=0, retried_nomax=0, retried_withmax=0):
"""Run a task and check the number of emails processed.""" """Run a task and check the number of emails processed."""
task_entry = self._create_input_entry() task_entry = self._create_input_entry()
parent_status = self._run_task_with_mock_celery(task_class, task_entry.id, task_entry.task_id) parent_status = self._run_task_with_mock_celery(task_class, task_entry.id, task_entry.task_id)
...@@ -238,7 +242,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): ...@@ -238,7 +242,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
# mark some students as opting out # mark some students as opting out
with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
get_conn.return_value.send_messages.side_effect = cycle([None]) get_conn.return_value.send_messages.side_effect = cycle([None])
self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, expected_succeeds, skipped=expected_skipped) self._test_run_with_task(
send_bulk_course_email, 'emailed', num_emails, expected_succeeds, skipped=expected_skipped
)
def _test_email_address_failures(self, exception): def _test_email_address_failures(self, exception):
"""Test that celery handles bad address errors by failing and not retrying.""" """Test that celery handles bad address errors by failing and not retrying."""
...@@ -251,7 +257,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): ...@@ -251,7 +257,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn: with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
# have every fourth email fail due to some address failure: # have every fourth email fail due to some address failure:
get_conn.return_value.send_messages.side_effect = cycle([exception, None, None, None]) get_conn.return_value.send_messages.side_effect = cycle([exception, None, None, None])
self._test_run_with_task(send_bulk_course_email, 'emailed', num_emails, expected_succeeds, failed=expected_fails) self._test_run_with_task(
send_bulk_course_email, 'emailed', num_emails, expected_succeeds, failed=expected_fails
)
def test_smtp_blacklisted_user(self): def test_smtp_blacklisted_user(self):
# Test that celery handles permanent SMTPDataErrors by failing and not retrying. # Test that celery handles permanent SMTPDataErrors by failing and not retrying.
...@@ -329,10 +337,14 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): ...@@ -329,10 +337,14 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
self._test_max_retry_limit_causes_failure(SMTPConnectError(424, "Bad Connection")) self._test_max_retry_limit_causes_failure(SMTPConnectError(424, "Bad Connection"))
def test_retry_after_aws_connect_error(self): def test_retry_after_aws_connect_error(self):
self._test_retry_after_limited_retry_error(AWSConnectionError("Unable to provide secure connection through proxy")) self._test_retry_after_limited_retry_error(
AWSConnectionError("Unable to provide secure connection through proxy")
)
def test_max_retry_after_aws_connect_error(self): def test_max_retry_after_aws_connect_error(self):
self._test_max_retry_limit_causes_failure(AWSConnectionError("Unable to provide secure connection through proxy")) self._test_max_retry_limit_causes_failure(
AWSConnectionError("Unable to provide secure connection through proxy")
)
def test_retry_after_general_error(self): def test_retry_after_general_error(self):
self._test_retry_after_limited_retry_error(Exception("This is some random exception.")) self._test_retry_after_limited_retry_error(Exception("This is some random exception."))
...@@ -371,7 +383,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): ...@@ -371,7 +383,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
self._test_retry_after_unlimited_retry_error(SMTPDataError(455, "Throttling: Sending rate exceeded")) self._test_retry_after_unlimited_retry_error(SMTPDataError(455, "Throttling: Sending rate exceeded"))
def test_retry_after_ses_throttling_error(self): def test_retry_after_ses_throttling_error(self):
self._test_retry_after_unlimited_retry_error(SESMaxSendingRateExceededError(455, "Throttling: Sending rate exceeded")) self._test_retry_after_unlimited_retry_error(
SESMaxSendingRateExceededError(455, "Throttling: Sending rate exceeded")
)
def _test_immediate_failure(self, exception): def _test_immediate_failure(self, exception):
"""Test that celery can hit a maximum number of retries.""" """Test that celery can hit a maximum number of retries."""
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment