Commit 296167e5 by Adam

Merge pull request #6950 from edx/release

Release
parents 5ccf6d53 f3f6aef9
...@@ -185,7 +185,8 @@ def _preview_module_system(request, descriptor, field_data): ...@@ -185,7 +185,8 @@ def _preview_module_system(request, descriptor, field_data):
wrappers=wrappers, wrappers=wrappers,
error_descriptor_class=ErrorDescriptor, error_descriptor_class=ErrorDescriptor,
get_user_role=lambda: get_user_role(request.user, course_id), get_user_role=lambda: get_user_role(request.user, course_id),
descriptor_runtime=descriptor.runtime, # Get the raw DescriptorSystem, not the CombinedSystem
descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access
services={ services={
"i18n": ModuleI18nService(), "i18n": ModuleI18nService(),
"field-data": field_data, "field-data": field_data,
......
...@@ -1382,7 +1382,7 @@ def _do_create_account(post_vars, extended_profile=None): ...@@ -1382,7 +1382,7 @@ def _do_create_account(post_vars, extended_profile=None):
return (user, profile, registration) return (user, profile, registration)
@ensure_csrf_cookie @csrf_exempt
def create_account(request, post_override=None): # pylint: disable-msg=too-many-statements def create_account(request, post_override=None): # pylint: disable-msg=too-many-statements
""" """
JSON call to create new edX account. JSON call to create new edX account.
......
...@@ -95,7 +95,7 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')): ...@@ -95,7 +95,7 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')):
""" """
user = Mock(name='get_test_system.user', is_staff=False) user = Mock(name='get_test_system.user', is_staff=False)
descriptor_system = get_test_descriptor_system(), descriptor_system = get_test_descriptor_system()
def get_module(descriptor): def get_module(descriptor):
"""Mocks module_system get_module function""" """Mocks module_system get_module function"""
...@@ -108,7 +108,7 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')): ...@@ -108,7 +108,7 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')):
# Descriptors can all share a single DescriptorSystem. # Descriptors can all share a single DescriptorSystem.
# So, bind to the same one as the current descriptor. # So, bind to the same one as the current descriptor.
module_system.descriptor_runtime = descriptor.runtime._descriptor_system module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access
descriptor.bind_for_student(module_system, descriptor._field_data) descriptor.bind_for_student(module_system, descriptor._field_data)
......
...@@ -54,14 +54,14 @@ class LibraryContentTest(MixedSplitTestCase): ...@@ -54,14 +54,14 @@ class LibraryContentTest(MixedSplitTestCase):
Bind a module (part of self.course) so we can access student-specific data. Bind a module (part of self.course) so we can access student-specific data.
""" """
module_system = get_test_system(course_id=self.course.location.course_key) module_system = get_test_system(course_id=self.course.location.course_key)
module_system.descriptor_runtime = module.runtime module_system.descriptor_runtime = module.runtime._descriptor_system # pylint: disable=protected-access
module_system._services['library_tools'] = self.tools # pylint: disable=protected-access module_system._services['library_tools'] = self.tools # pylint: disable=protected-access
def get_module(descriptor): def get_module(descriptor):
"""Mocks module_system get_module function""" """Mocks module_system get_module function"""
sub_module_system = get_test_system(course_id=self.course.location.course_key) sub_module_system = get_test_system(course_id=self.course.location.course_key)
sub_module_system.get_module = get_module sub_module_system.get_module = get_module
sub_module_system.descriptor_runtime = descriptor.runtime sub_module_system.descriptor_runtime = descriptor._runtime # pylint: disable=protected-access
descriptor.bind_for_student(sub_module_system, descriptor._field_data) # pylint: disable=protected-access descriptor.bind_for_student(sub_module_system, descriptor._field_data) # pylint: disable=protected-access
return descriptor return descriptor
......
...@@ -78,7 +78,7 @@ class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase): ...@@ -78,7 +78,7 @@ class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase):
self.course_sequence = self.course.get_children()[0] self.course_sequence = self.course.get_children()[0]
self.module_system = get_test_system() self.module_system = get_test_system()
self.module_system.descriptor_runtime = self.course.runtime._descriptor_system # pylint: disable=protected-access self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access
self.course.runtime.export_fs = MemoryFS() self.course.runtime.export_fs = MemoryFS()
self.partitions_service = StaticPartitionService( self.partitions_service = StaticPartitionService(
......
...@@ -28,7 +28,7 @@ class BaseVerticalModuleTest(XModuleXmlImportTest): ...@@ -28,7 +28,7 @@ class BaseVerticalModuleTest(XModuleXmlImportTest):
course_seq = self.course.get_children()[0] course_seq = self.course.get_children()[0]
self.module_system = get_test_system() self.module_system = get_test_system()
self.module_system.descriptor_runtime = self.course.runtime._descriptor_system # pylint: disable=protected-access self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access
self.course.runtime.export_fs = MemoryFS() self.course.runtime.export_fs = MemoryFS()
self.vertical = course_seq.get_children()[0] self.vertical = course_seq.get_children()[0]
......
...@@ -3,6 +3,7 @@ import os ...@@ -3,6 +3,7 @@ import os
import sys import sys
import yaml import yaml
from contracts import contract, new_contract
from functools import partial from functools import partial
from lxml import etree from lxml import etree
from collections import namedtuple from collections import namedtuple
...@@ -1307,6 +1308,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p ...@@ -1307,6 +1308,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p
pass pass
new_contract('DescriptorSystem', DescriptorSystem)
class XMLParsingSystem(DescriptorSystem): class XMLParsingSystem(DescriptorSystem):
def __init__(self, process_xml, **kwargs): def __init__(self, process_xml, **kwargs):
""" """
...@@ -1427,6 +1431,8 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylin ...@@ -1427,6 +1431,8 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylin
Note that these functions can be closures over e.g. a django request Note that these functions can be closures over e.g. a django request
and user, or other environment-specific info. and user, or other environment-specific info.
""" """
@contract(descriptor_runtime='DescriptorSystem')
def __init__( def __init__(
self, static_url, track_function, get_module, render_template, self, static_url, track_function, get_module, render_template,
replace_urls, descriptor_runtime, user=None, filestore=None, replace_urls, descriptor_runtime, user=None, filestore=None,
......
...@@ -6,6 +6,7 @@ import re ...@@ -6,6 +6,7 @@ import re
import random import random
import json import json
from time import sleep from time import sleep
from collections import Counter
import dogstats_wrapper as dog_stats_api import dogstats_wrapper as dog_stats_api
from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPException from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPException
...@@ -418,12 +419,31 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas ...@@ -418,12 +419,31 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
'failed' count above. 'failed' count above.
""" """
# Get information from current task's request: # Get information from current task's request:
parent_task_id = InstructorTask.objects.get(pk=entry_id).task_id
task_id = subtask_status.task_id task_id = subtask_status.task_id
total_recipients = len(to_list)
recipient_num = 0
total_recipients_successful = 0
total_recipients_failed = 0
recipients_info = Counter()
log.info(
"BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, TotalRecipients: %s",
parent_task_id,
task_id,
email_id,
total_recipients
)
try: try:
course_email = CourseEmail.objects.get(id=email_id) course_email = CourseEmail.objects.get(id=email_id)
except CourseEmail.DoesNotExist as exc: except CourseEmail.DoesNotExist as exc:
log.exception("Task %s: could not find email id:%s to send.", task_id, email_id) log.exception(
"BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Could not find email to send.",
parent_task_id,
task_id,
email_id
)
raise raise
# Exclude optouts (if not a retry): # Exclude optouts (if not a retry):
...@@ -459,6 +479,7 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas ...@@ -459,6 +479,7 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
# That way, the to_list will always contain the recipients remaining to be emailed. # That way, the to_list will always contain the recipients remaining to be emailed.
# This is convenient for retries, which will need to send to those who haven't # This is convenient for retries, which will need to send to those who haven't
# yet been emailed, but not send to those who have already been sent to. # yet been emailed, but not send to those who have already been sent to.
recipient_num += 1
current_recipient = to_list[-1] current_recipient = to_list[-1]
email = current_recipient['email'] email = current_recipient['email']
email_context['email'] = email email_context['email'] = email
...@@ -489,29 +510,81 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas ...@@ -489,29 +510,81 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
sleep(settings.BULK_EMAIL_RETRY_DELAY_BETWEEN_SENDS) sleep(settings.BULK_EMAIL_RETRY_DELAY_BETWEEN_SENDS)
try: try:
log.debug('Email with id %s to be sent to %s', email_id, email) log.info(
"BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Recipient num: %s/%s, \
Recipient name: %s, Email address: %s",
parent_task_id,
task_id,
email_id,
recipient_num,
total_recipients,
current_recipient['profile__name'],
email
)
with dog_stats_api.timer('course_email.single_send.time.overall', tags=[_statsd_tag(course_title)]): with dog_stats_api.timer('course_email.single_send.time.overall', tags=[_statsd_tag(course_title)]):
connection.send_messages([email_msg]) connection.send_messages([email_msg])
except SMTPDataError as exc: except SMTPDataError as exc:
# According to SMTP spec, we'll retry error codes in the 4xx range. 5xx range indicates hard failure. # According to SMTP spec, we'll retry error codes in the 4xx range. 5xx range indicates hard failure.
total_recipients_failed += 1
log.error(
"BulkEmail ==> Status: Failed(SMTPDataError), Task: %s, SubTask: %s, EmailId: %s, \
Recipient num: %s/%s, Email address: %s",
parent_task_id,
task_id,
email_id,
recipient_num,
total_recipients,
email
)
if exc.smtp_code >= 400 and exc.smtp_code < 500: if exc.smtp_code >= 400 and exc.smtp_code < 500:
# This will cause the outer handler to catch the exception and retry the entire task. # This will cause the outer handler to catch the exception and retry the entire task.
raise exc raise exc
else: else:
# This will fall through and not retry the message. # This will fall through and not retry the message.
log.warning('Task %s: email with id %s not delivered to %s due to error %s', task_id, email_id, email, exc.smtp_error) log.warning(
'BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Recipient num: %s/%s, \
Email not delivered to %s due to error %s',
parent_task_id,
task_id,
email_id,
recipient_num,
total_recipients,
email,
exc.smtp_error
)
dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)]) dog_stats_api.increment('course_email.error', tags=[_statsd_tag(course_title)])
subtask_status.increment(failed=1) subtask_status.increment(failed=1)
except SINGLE_EMAIL_FAILURE_ERRORS as exc: except SINGLE_EMAIL_FAILURE_ERRORS as exc:
# This will fall through and not retry the message. # This will fall through and not retry the message.
log.warning('Task %s: email with id %s not delivered to %s due to error %s', task_id, email_id, email, exc) total_recipients_failed += 1
log.error(
"BulkEmail ==> Status: Failed(SINGLE_EMAIL_FAILURE_ERRORS), Task: %s, SubTask: %s, \
EmailId: %s, Recipient num: %s/%s, Email address: %s, Exception: %s",
parent_task_id,
task_id,
email_id,
recipient_num,
total_recipients,
email,
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)])
subtask_status.increment(failed=1) subtask_status.increment(failed=1)
else: else:
total_recipients_successful += 1
log.info(
"BulkEmail ==> Status: Success, Task: %s, SubTask: %s, EmailId: %s, \
Recipient num: %s/%s, Email address: %s,",
parent_task_id,
task_id,
email_id,
recipient_num,
total_recipients,
email
)
dog_stats_api.increment('course_email.sent', tags=[_statsd_tag(course_title)]) dog_stats_api.increment('course_email.sent', tags=[_statsd_tag(course_title)])
if settings.BULK_EMAIL_LOG_SENT_EMAILS: if settings.BULK_EMAIL_LOG_SENT_EMAILS:
log.info('Email with id %s sent to %s', email_id, email) log.info('Email with id %s sent to %s', email_id, email)
...@@ -522,8 +595,32 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas ...@@ -522,8 +595,32 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
# Pop the user that was emailed off the end of the list only once they have # Pop the user that was emailed off the end of the list only once they have
# successfully been processed. (That way, if there were a failure that # successfully been processed. (That way, if there were a failure that
# needed to be retried, the user is still on the list.) # needed to be retried, the user is still on the list.)
recipients_info[email] += 1
to_list.pop() to_list.pop()
log.info(
"BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Total Successful Recipients: %s/%s, \
Failed Recipients: %s/%s",
parent_task_id,
task_id,
email_id,
total_recipients_successful,
total_recipients,
total_recipients_failed,
total_recipients
)
duplicate_recipients = ["{0} ({1})".format(email, repetition)
for email, repetition in recipients_info.most_common() if repetition > 1]
if duplicate_recipients:
log.info(
"BulkEmail ==> Task: %s, SubTask: %s, EmailId: %s, Total Duplicate Recipients [%s]: [%s]",
parent_task_id,
task_id,
email_id,
len(duplicate_recipients),
', '.join(duplicate_recipients)
)
except INFINITE_RETRY_ERRORS as exc: except INFINITE_RETRY_ERRORS as exc:
dog_stats_api.increment('course_email.infinite_retry', tags=[_statsd_tag(course_title)]) dog_stats_api.increment('course_email.infinite_retry', tags=[_statsd_tag(course_title)])
# Increment the "retried_nomax" counter, update other counters with progress to date, # Increment the "retried_nomax" counter, update other counters with progress to date,
......
...@@ -642,7 +642,7 @@ def get_module_system_for_user(user, field_data_cache, ...@@ -642,7 +642,7 @@ def get_module_system_for_user(user, field_data_cache,
'user': DjangoXBlockUserService(user, user_is_staff=user_is_staff), 'user': DjangoXBlockUserService(user, user_is_staff=user_is_staff),
}, },
get_user_role=lambda: get_user_role(user, course_id), get_user_role=lambda: get_user_role(user, course_id),
descriptor_runtime=descriptor.runtime, descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access
rebind_noauth_module_to_user=rebind_noauth_module_to_user, rebind_noauth_module_to_user=rebind_noauth_module_to_user,
user_location=user_location, user_location=user_location,
request_token=request_token, request_token=request_token,
......
...@@ -41,7 +41,7 @@ from xmodule.modulestore import ModuleStoreEnum ...@@ -41,7 +41,7 @@ from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory, check_mongo_calls from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory, check_mongo_calls
from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW, CombinedSystem
TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT
...@@ -892,13 +892,16 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -892,13 +892,16 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase):
_field_data=Mock(spec=FieldData), _field_data=Mock(spec=FieldData),
location=location, location=location,
static_asset_path=None, static_asset_path=None,
runtime=Mock( _runtime=Mock(
spec=Runtime, spec=Runtime,
resources_fs=None, resources_fs=None,
mixologist=Mock(_mixins=()) mixologist=Mock(_mixins=(), name='mixologist'),
name='runtime',
), ),
scope_ids=Mock(spec=ScopeIds), scope_ids=Mock(spec=ScopeIds),
name='descriptor'
) )
descriptor.runtime = CombinedSystem(descriptor._runtime, None) # pylint: disable=protected-access
# Use the xblock_class's bind_for_student method # Use the xblock_class's bind_for_student method
descriptor.bind_for_student = partial(xblock_class.bind_for_student, descriptor) descriptor.bind_for_student = partial(xblock_class.bind_for_student, descriptor)
...@@ -908,10 +911,10 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase): ...@@ -908,10 +911,10 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase):
return render.get_module_for_descriptor_internal( return render.get_module_for_descriptor_internal(
user=self.user, user=self.user,
descriptor=descriptor, descriptor=descriptor,
field_data_cache=Mock(spec=FieldDataCache), field_data_cache=Mock(spec=FieldDataCache, name='field_data_cache'),
course_id=course_id, course_id=course_id,
track_function=Mock(), # Track Function track_function=Mock(name='track_function'), # Track Function
xqueue_callback_url_prefix=Mock(), # XQueue Callback Url Prefix xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'), # XQueue Callback Url Prefix
request_token='request_token', request_token='request_token',
).xmodule_runtime.anonymous_student_id ).xmodule_runtime.anonymous_student_id
......
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