diff --git a/lms/djangoapps/bulk_email/admin.py b/lms/djangoapps/bulk_email/admin.py index 10b3c53f87..a17699f500 100644 --- a/lms/djangoapps/bulk_email/admin.py +++ b/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 diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index 0842f93958..b693f9503a 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/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. diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 3c75d2fe3a..5fa816f523 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/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), diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email/tests/test_err_handling.py index 9b8db791d9..89f1195aea 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/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 diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email/tests/test_forms.py index e9b03760d9..13180d4e52 100644 --- a/lms/djangoapps/bulk_email/tests/test_forms.py +++ b/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() diff --git a/lms/djangoapps/bulk_email/tests/test_models.py b/lms/djangoapps/bulk_email/tests/test_models.py index 5949287f3a..29433c9877 100644 --- a/lms/djangoapps/bulk_email/tests/test_models.py +++ b/lms/djangoapps/bulk_email/tests/test_models.py @@ -40,7 +40,9 @@ class CourseEmailTest(TestCase): html_message = "dummy message" 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) diff --git a/lms/djangoapps/bulk_email/tests/test_tasks.py b/lms/djangoapps/bulk_email/tests/test_tasks.py index 12442d3200..f732d5d5a8 100644 --- a/lms/djangoapps/bulk_email/tests/test_tasks.py +++ b/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", "

This is a test message

") + course_email = CourseEmail.create( + course_id, self.instructor, to_option, "Test Subject", "

This is a test message

" + ) 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."""