diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 0f9c7e9593aafa5ed87d76e7057bf3f8f7d32619..e9fff63698f4a9d4671e9bfa5806af7c550370cf 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -39,8 +39,7 @@ from instructor_task.api import (get_running_instructor_tasks, get_instructor_task_history, submit_rescore_problem_for_all_students, submit_rescore_problem_for_student, - submit_reset_problem_attempts_for_all_students, - submit_delete_problem_state_for_all_students) + submit_reset_problem_attempts_for_all_students) from instructor_task.views import get_task_completion_info from mitxmako.shortcuts import render_to_response from psychometrics import psychoanalyze @@ -1138,7 +1137,7 @@ def _do_unenroll_students(course_id, students): """Do the actual work of un-enrolling multiple students, presented as a string of emails separated by commas or returns""" - old_students, old_students_lc = get_and_clean_student_list(students) + old_students, _ = get_and_clean_student_list(students) status = dict([x, 'unprocessed'] for x in old_students) for student in old_students: @@ -1162,7 +1161,7 @@ def _do_unenroll_students(course_id, students): try: ce[0].delete() status[student] = "un-enrolled" - except Exception as err: + except Exception: if not isok: status[student] = "Error! Failed to un-enroll" @@ -1319,7 +1318,7 @@ def get_background_task_table(course_id, problem_url, student=None): "Task Id", "Requester", "Submitted", - "Duration (ms)", + "Duration (sec)", "Task State", "Task Status", "Task Output"] @@ -1327,11 +1326,11 @@ def get_background_task_table(course_id, problem_url, student=None): datatable['data'] = [] for instructor_task in history_entries: # get duration info, if known: - duration_ms = 'unknown' - if hasattr(instructor_task, 'task_output'): + duration_sec = 'unknown' + if hasattr(instructor_task, 'task_output') and instructor_task.task_output is not None: task_output = json.loads(instructor_task.task_output) if 'duration_ms' in task_output: - duration_ms = task_output['duration_ms'] + duration_sec = int(task_output['duration_ms'] / 1000.0) # get progress status message: success, task_message = get_task_completion_info(instructor_task) status = "Complete" if success else "Incomplete" @@ -1340,7 +1339,7 @@ def get_background_task_table(course_id, problem_url, student=None): str(instructor_task.task_id), str(instructor_task.requester), instructor_task.created.isoformat(' '), - duration_ms, + duration_sec, str(instructor_task.task_state), status, task_message] diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index 9776c7336d5265859733f52ee89a83c9f93b868f..62ef6296b6e0962d51b330473a8cac9907d49fc2 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -13,6 +13,7 @@ from traceback import format_exc from celery import current_task from celery.utils.log import get_task_logger +from celery.signals import worker_process_init from celery.states import SUCCESS, FAILURE from django.contrib.auth.models import User @@ -39,6 +40,26 @@ PROGRESS = 'PROGRESS' UNKNOWN_TASK_ID = 'unknown-task_id' +def initialize_mako(sender=None, conf=None, **kwargs): + """ + Get mako templates to work on celery worker server's worker thread. + + The initialization of Mako templating is usually done when Django is + initializing middleware packages as part of processing a server request. + When this is run on a celery worker server, no such initialization is + called. + + To make sure that we don't load this twice (just in case), we look for the + result: the defining of the lookup paths for templates. + """ + if 'main' not in middleware.lookup: + TASK_LOG.info("Initializing Mako middleware explicitly") + middleware.MakoMiddleware() + +# Actually make the call to define the hook: +worker_process_init.connect(initialize_mako) + + class UpdateProblemModuleStateError(Exception): """ Error signaling a fatal condition while updating problem modules. @@ -90,17 +111,6 @@ def _perform_module_state_update(course_id, module_state_key, student_identifier # get start time for task: start_time = time() - # Hack to get mako templates to work on celery worker server's worker thread. - # The initialization of Mako templating is usually done when Django is - # initializing middleware packages as part of processing a server request. - # When this is run on a celery worker server, no such initialization is - # called. Using @worker_ready.connect doesn't run in the right container. - # So we look for the result: the defining of the lookup paths - # for templates. - if 'main' not in middleware.lookup: - TASK_LOG.debug("Initializing Mako middleware explicitly") - middleware.MakoMiddleware() - # find the problem descriptor: module_descriptor = modulestore().get_instance(course_id, module_state_key) @@ -147,7 +157,7 @@ def _perform_module_state_update(course_id, module_state_key, student_identifier num_attempted += 1 # There is no try here: if there's an error, we let it throw, and the task will # be marked as FAILED, with a stack trace. - with dog_stats_api.timer('instructor_tasks.module.{0}.time'.format(action_name)): + with dog_stats_api.timer('instructor_tasks.module.time.step', tags=['action:{name}'.format(name=action_name)]): if update_fcn(module_descriptor, module_to_update, xmodule_instance_args): # If the update_fcn returns true, then it performed some kind of work. # Logging of failures is left to the update_fcn itself. @@ -232,7 +242,7 @@ def update_problem_module_state(entry_id, update_fcn, action_name, filter_fcn, raise UpdateProblemModuleStateError(message) # now do the work: - with dog_stats_api.timer('instructor_tasks.module.{0}.overall_time'.format(action_name)): + with dog_stats_api.timer('instructor_tasks.module.time.overall', tags=['action:{name}'.format(name=action_name)]): task_progress = _perform_module_state_update(course_id, module_state_key, student_ident, update_fcn, action_name, filter_fcn, xmodule_instance_args) except Exception: @@ -316,7 +326,6 @@ def rescore_problem_module_state(module_descriptor, student_module, xmodule_inst course_id = student_module.course_id student = student_module.student module_state_key = student_module.module_state_key - instance = _get_module_instance_for_task(course_id, student, module_descriptor, xmodule_instance_args, grade_bucket_type='rescore') if instance is None: diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index c1b87865b70aea084a78d89219dc7cc9e07b79c9..9458a27498b6bc8b91358795409166baf4d4496a 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -211,11 +211,11 @@ class InstructorTaskReportTest(TestCase): 'total': 10, 'action_name': 'rescored'} output = self._test_get_status_from_result(task_id, mock_result) + self.assertEquals(output['message'], "Progress: rescored 4 of 5 so far (out of 10)") + self.assertEquals(output['succeeded'], False) self.assertEquals(output['task_state'], PROGRESS) self.assertTrue(output['in_progress']) self.assertEquals(output['task_progress'], mock_result.result) - for key in ['message', 'succeeded']: - self.assertTrue(key not in output) def test_update_progress_to_failure(self): # view task entry for task in progress that later fails diff --git a/lms/djangoapps/instructor_task/views.py b/lms/djangoapps/instructor_task/views.py index 6e49dc1421d42fb5ab73e87716607dd98743624a..77fb09096e522ef0b761325bde937c1a8051353f 100644 --- a/lms/djangoapps/instructor_task/views.py +++ b/lms/djangoapps/instructor_task/views.py @@ -8,10 +8,14 @@ from celery.states import FAILURE, REVOKED, READY_STATES from instructor_task.api_helper import (get_status_from_instructor_task, get_updated_instructor_task) +from instructor_task.tasks_helper import PROGRESS log = logging.getLogger(__name__) +# return status for completed tasks and tasks in progress +STATES_WITH_STATUS = [state for state in READY_STATES] + [PROGRESS] + def instructor_task_status(request): """ @@ -32,10 +36,11 @@ def instructor_task_status(request): Task_id values that are unrecognized are skipped. The dict with status information for a task contains the following keys: - 'message': on complete tasks, status message reporting on final progress, - or providing exception message if failed. - 'succeeded': on complete tasks, indicates if the task outcome was successful: - did it achieve what it set out to do. + 'message': on complete tasks, status message reporting on final progress, + or providing exception message if failed. For tasks in progress, + indicates the current progress. + 'succeeded': on complete tasks or tasks in progress, indicates if the + task outcome was successful: did it achieve what it set out to do. This is in contrast with a successful task_state, which indicates that the task merely completed. 'task_id': id assigned by LMS and used by celery. @@ -62,7 +67,7 @@ def instructor_task_status(request): """ instructor_task = get_updated_instructor_task(task_id) status = get_status_from_instructor_task(instructor_task) - if instructor_task.task_state in READY_STATES: + if instructor_task.task_state in STATES_WITH_STATUS: succeeded, message = get_task_completion_info(instructor_task) status['message'] = message status['succeeded'] = succeeded @@ -97,8 +102,7 @@ def get_task_completion_info(instructor_task): """ succeeded = False - # if still in progress, then we assume there is no completion info to provide: - if instructor_task.task_state not in READY_STATES: + if instructor_task.task_state not in STATES_WITH_STATUS: return (succeeded, "No status information available") # we're more surprised if there is no output for a completed task, but just warn: @@ -135,7 +139,10 @@ def get_task_completion_info(instructor_task): else: student = task_input.get('student') - if student is not None: + if instructor_task.task_state == PROGRESS: + # special message for providing progress updates: + msg_format = "Progress: {action} {updated} of {attempted} so far" + elif student is not None: if num_attempted == 0: msg_format = "Unable to find submission to be {action} for student '{student}'" elif num_updated == 0: diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 4c9b4200a284fed9e5df01e2997fcc5fc6f34b0f..ea78cca791aba406074f7e24b841c5e7bfcbd4d9 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -26,14 +26,6 @@ this.InstructorTaskProgress = (function() { - // Hardcode the refresh interval to be every five seconds. - // TODO: allow the refresh interval to be set. (And if it is disabled, - // then don't set the timeout at all.) - var refresh_interval = 5000; - - // Hardcode the initial delay before the first refresh to two seconds: - var initial_refresh_delay = 2000; - function InstructorTaskProgress(element) { this.update_progress = __bind(this.update_progress, this); this.get_status = __bind(this.get_status, this); @@ -42,7 +34,8 @@ if (window.queuePollerID) { window.clearTimeout(window.queuePollerID); } - window.queuePollerID = window.setTimeout(this.get_status, this.initial_refresh_delay); + // Hardcode the initial delay before the first refresh to one second: + window.queuePollerID = window.setTimeout(this.get_status, 1000); } InstructorTaskProgress.prototype.$ = function(selector) { @@ -60,7 +53,8 @@ // find the corresponding entry, and update it: entry = $(_this.element).find('[data-task-id="' + task_id + '"]'); entry.find('.task-state').text(task_dict.task_state) - var duration_value = (task_dict.task_progress && task_dict.task_progress.duration_ms) || 'unknown'; + var duration_value = (task_dict.task_progress && task_dict.task_progress.duration_ms + && Math.round(task_dict.task_progress.duration_ms/1000)) || 'unknown'; entry.find('.task-duration').text(duration_value); var progress_value = task_dict.message || ''; entry.find('.task-progress').text(progress_value); @@ -74,8 +68,11 @@ } // if some entries are still incomplete, then repoll: + // Hardcode the refresh interval to be every five seconds. + // TODO: allow the refresh interval to be set. (And if it is disabled, + // then don't set the timeout at all.) if (something_in_progress) { - window.queuePollerID = window.setTimeout(_this.get_status, _this.refresh_interval); + window.queuePollerID = window.setTimeout(_this.get_status, 5000); } else { delete window.queuePollerID; } @@ -741,7 +738,7 @@ function goto( mode) <th>Requester</th> <th>Submitted</th> <th>Task State</th> - <th>Duration (ms)</th> + <th>Duration (sec)</th> <th>Task Progress</th> </tr> %for tasknum, instructor_task in enumerate(instructor_tasks):