Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get the raw DescriptorSystem, not the CombinedSystem when instantiating ... #6930

Merged
merged 4 commits into from Feb 10, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion cms/djangoapps/contentstore/views/preview.py
Expand Up @@ -185,7 +185,8 @@ def _preview_module_system(request, descriptor, field_data):
wrappers=wrappers,
error_descriptor_class=ErrorDescriptor,
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={
"i18n": ModuleI18nService(),
"field-data": field_data,
Expand Down
2 changes: 1 addition & 1 deletion common/djangoapps/student/views.py
Expand Up @@ -1370,7 +1370,7 @@ def _do_create_account(post_vars, extended_profile=None):
return (user, profile, registration)


@ensure_csrf_cookie
@csrf_exempt
def create_account(request, post_override=None): # pylint: disable-msg=too-many-statements
"""
JSON call to create new edX account.
Expand Down
4 changes: 2 additions & 2 deletions common/lib/xmodule/xmodule/tests/__init__.py
Expand Up @@ -94,7 +94,7 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')):
"""
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):
"""Mocks module_system get_module function"""
Expand All @@ -107,7 +107,7 @@ def get_module(descriptor):

# Descriptors can all share a single DescriptorSystem.
# 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)

Expand Down
4 changes: 2 additions & 2 deletions common/lib/xmodule/xmodule/tests/test_library_content.py
Expand Up @@ -54,14 +54,14 @@ def _bind_course_module(self, module):
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.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

def get_module(descriptor):
"""Mocks module_system get_module function"""
sub_module_system = get_test_system(course_id=self.course.location.course_key)
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
return descriptor

Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/tests/test_split_test_module.py
Expand Up @@ -78,7 +78,7 @@ def setUp(self):
self.course_sequence = self.course.get_children()[0]
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.partitions_service = StaticPartitionService(
Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/tests/test_vertical.py
Expand Up @@ -27,7 +27,7 @@ def setUp(self):
course_seq = self.course.get_children()[0]
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.vertical = course_seq.get_children()[0]
Expand Down
6 changes: 6 additions & 0 deletions common/lib/xmodule/xmodule/x_module.py
Expand Up @@ -3,6 +3,7 @@
import sys
import yaml

from contracts import contract, new_contract
from functools import partial
from lxml import etree
from collections import namedtuple
Expand Down Expand Up @@ -1307,6 +1308,9 @@ def publish(self, block, event_type, event):
pass


new_contract('DescriptorSystem', DescriptorSystem)


class XMLParsingSystem(DescriptorSystem):
def __init__(self, process_xml, **kwargs):
"""
Expand Down Expand Up @@ -1427,6 +1431,8 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # pylin
Note that these functions can be closures over e.g. a django request
and user, or other environment-specific info.
"""

@contract(descriptor_runtime='DescriptorSystem')
def __init__(
self, static_url, track_function, get_module, render_template,
replace_urls, descriptor_runtime, user=None, filestore=None,
Expand Down
107 changes: 102 additions & 5 deletions lms/djangoapps/bulk_email/tasks.py
Expand Up @@ -6,6 +6,7 @@
import random
import json
from time import sleep
from collections import Counter

import dogstats_wrapper as dog_stats_api
from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPException
Expand Down Expand Up @@ -418,12 +419,31 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
'failed' count above.
"""
# Get information from current task's request:
parent_task_id = InstructorTask.objects.get(pk=entry_id).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:
course_email = CourseEmail.objects.get(id=email_id)
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

# Exclude optouts (if not a retry):
Expand Down Expand Up @@ -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.
# 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.
recipient_num += 1
current_recipient = to_list[-1]
email = current_recipient['email']
email_context['email'] = email
Expand Down Expand Up @@ -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)

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)]):
connection.send_messages([email_msg])

except SMTPDataError as exc:
# 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:
# This will cause the outer handler to catch the exception and retry the entire task.
raise exc
else:
# 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)])
subtask_status.increment(failed=1)

except SINGLE_EMAIL_FAILURE_ERRORS as exc:
# 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)])
subtask_status.increment(failed=1)

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)])
if settings.BULK_EMAIL_LOG_SENT_EMAILS:
log.info('Email with id %s sent to %s', email_id, email)
Expand All @@ -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
# successfully been processed. (That way, if there were a failure that
# needed to be retried, the user is still on the list.)
recipients_info[email] += 1
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:
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,
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/module_render.py
Expand Up @@ -642,7 +642,7 @@ def rebind_noauth_module_to_user(module, real_user):
'user': DjangoXBlockUserService(user, user_is_staff=user_is_staff),
},
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,
user_location=user_location,
request_token=request_token,
Expand Down
15 changes: 9 additions & 6 deletions lms/djangoapps/courseware/tests/test_module_render.py
Expand Up @@ -41,7 +41,7 @@
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
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

Expand Down Expand Up @@ -903,13 +903,16 @@ def _get_anonymous_id(self, course_id, xblock_class):
_field_data=Mock(spec=FieldData),
location=location,
static_asset_path=None,
runtime=Mock(
_runtime=Mock(
spec=Runtime,
resources_fs=None,
mixologist=Mock(_mixins=())
mixologist=Mock(_mixins=(), name='mixologist'),
name='runtime',
),
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
descriptor.bind_for_student = partial(xblock_class.bind_for_student, descriptor)

Expand All @@ -919,10 +922,10 @@ def _get_anonymous_id(self, course_id, xblock_class):
return render.get_module_for_descriptor_internal(
user=self.user,
descriptor=descriptor,
field_data_cache=Mock(spec=FieldDataCache),
field_data_cache=Mock(spec=FieldDataCache, name='field_data_cache'),
course_id=course_id,
track_function=Mock(), # Track Function
xqueue_callback_url_prefix=Mock(), # XQueue Callback Url Prefix
track_function=Mock(name='track_function'), # Track Function
xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'), # XQueue Callback Url Prefix
request_token='request_token',
).xmodule_runtime.anonymous_student_id

Expand Down