Skip to content

E
edx-platform
Commit 927dc102 by Matt Drayer
Options

Merge pull request #10169 from mitocw/gdm_fix_pylint_lms.djangoapps.bulk_email 

Fixed pylint violation for files in lms/djangoapps/bulk_email
parents 744c1025 08dfe5e9
Showing with 69 additions and 29 deletions
lms/djangoapps/bulk_email/admin.py
......@@ -54,7 +54,9 @@ unsupported tags will cause email sending to fail.
return True
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
......
lms/djangoapps/bulk_email/models.py
......@@ -75,7 +75,9 @@ class CourseEmail(Email):
return self.subject
@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.
......
lms/djangoapps/bulk_email/tasks.py
......@@ -25,9 +25,9 @@ from boto.ses.exceptions import (
)
from boto.exception import AWSConnectionError
from celery import task, current_task
from celery.states import SUCCESS, FAILURE, RETRY
from celery.exceptions import RetryTaskError
from celery import task, current_task # pylint: disable=no-name-in-module
from celery.states import SUCCESS, FAILURE, RETRY # pylint: disable=no-name-in-module, import-error
from celery.exceptions import RetryTaskError # pylint: disable=no-name-in-module, import-error
from django.conf import settings
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
subtask_status = SubtaskStatus.from_dict(subtask_status_dict)
current_task_id = subtask_status.task_id
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)
# 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
except BULK_EMAIL_FAILURE_ERRORS as exc:
dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)])
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)
# Update counters with progress to date, counting unsent emails as failures,
# and set the state to FAILURE:
subtask_status.increment(failed=num_pending, state=FAILURE)
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
# 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
# succeed on retry, we give them a retry.
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)
# Increment the "retried_withmax" counter, update other counters with progress to date,
# and set the state to RETRY:
......@@ -709,7 +712,8 @@ def _get_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.
......@@ -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.
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)
# 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
log.exception(u'Task %s: email with id %d caused send_course_email task to retry.',
task_id, email_id)
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,
# 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),
......
lms/djangoapps/bulk_email/tests/test_err_handling.py
......@@ -4,7 +4,7 @@ Unit tests for handling email sending errors
"""
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.core.management import call_command
from django.core.urlresolvers import reverse
......
lms/djangoapps/bulk_email/tests/test_forms.py
......@@ -60,7 +60,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
"Course authorization with this Course id already exists.",
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()
# Course should still be authorized (invalid attempt had no effect)
......@@ -81,7 +84,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
msg += 'Please recheck that you have supplied a valid course id.'
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()
@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
......@@ -96,7 +102,10 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
msg += 'Please recheck that you have supplied a valid course id.'
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()
@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
......@@ -107,11 +116,14 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
# Validation shouldn't work
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'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()
......@@ -134,7 +146,10 @@ class CourseAuthorizationXMLFormTest(ModuleStoreTestCase):
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
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()
......
lms/djangoapps/bulk_email/tests/test_models.py
......@@ -40,7 +40,9 @@ class CourseEmailTest(TestCase):
html_message = "<html>dummy message</html>"
template_name = "branded_template"
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.to_option, SEND_TO_STAFF)
self.assertEquals(email.subject, subject)
......
lms/djangoapps/bulk_email/tests/test_tasks.py
......@@ -25,7 +25,7 @@ from boto.ses.exceptions import (
)
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.core.management import call_command
......@@ -96,7 +96,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
"""
to_option = SEND_TO_ALL
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_id = str(uuid4())
instructor_task = InstructorTaskFactory.create(
......@@ -153,7 +155,7 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
self.assertEquals(len(task_id_list), 1)
task_id = task_id_list[0]
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('attempted'), succeeded + failed)
self.assertEquals(subtask_status.get('succeeded'), succeeded)
......@@ -163,7 +165,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
self.assertEquals(subtask_status.get('retried_withmax'), retried_withmax)
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."""
task_entry = self._create_input_entry()
parent_status = self._run_task_with_mock_celery(task_class, task_entry.id, task_entry.task_id)
......@@ -238,7 +242,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
# mark some students as opting out
with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
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):
"""Test that celery handles bad address errors by failing and not retrying."""
......@@ -251,7 +257,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
with patch('bulk_email.tasks.get_connection', autospec=True) as get_conn:
# have every fourth email fail due to some address failure:
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):
# Test that celery handles permanent SMTPDataErrors by failing and not retrying.
......@@ -329,10 +337,14 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
self._test_max_retry_limit_causes_failure(SMTPConnectError(424, "Bad Connection"))
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):
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):
self._test_retry_after_limited_retry_error(Exception("This is some random exception."))
......@@ -371,7 +383,9 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
self._test_retry_after_unlimited_retry_error(SMTPDataError(455, "Throttling: Sending rate exceeded"))
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):
"""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