tasks now extract task_input and course_id from InstructorTask, instead of passing explicitly.

lms/djangoapps/instructor_task/api_helper.py

@@ -277,7 +277,7 @@ def submit_task(request, task_type, task_class, course_id, task_input, task_key)
 
     # submit task:
     task_id = instructor_task.task_id
-    task_args = [instructor_task.id, course_id, task_input, _get_xmodule_instance_args(request)]
+    task_args = [instructor_task.id, _get_xmodule_instance_args(request)]
     task_class.apply_async(task_args, task_id=task_id)
 
     return instructor_task

lms/djangoapps/instructor_task/tasks.py

 """
-This file contains tasks that are designed to perform background operations on the 
+This file contains tasks that are designed to perform background operations on the
 running state of a course.
 
 
 
 """
 from celery import task
-from instructor_task.tasks_helper import (_update_problem_module_state,
-                                          _rescore_problem_module_state,
-                                          _reset_problem_attempts_module_state,
-                                          _delete_problem_module_state)
+from instructor_task.tasks_helper import (update_problem_module_state,
+                                          rescore_problem_module_state,
+                                          reset_attempts_module_state,
+                                          delete_problem_module_state)
 
 
 @task
-def rescore_problem(entry_id, course_id, task_input, xmodule_instance_args):
+def rescore_problem(entry_id, xmodule_instance_args):
     """Rescores problem in `course_id`.
 
     `entry_id` is the id value of the InstructorTask entry that corresponds to this task.
@@ -29,19 +29,15 @@ def rescore_problem(entry_id, course_id, task_input, xmodule_instance_args):
     to instantiate an xmodule instance.
     """
     action_name = 'rescored'
-    update_fcn = _rescore_problem_module_state
+    update_fcn = rescore_problem_module_state
     filter_fcn = lambda(modules_to_update): modules_to_update.filter(state__contains='"done": true')
-    problem_url = task_input.get('problem_url')
-    student_ident = None
-    if 'student' in task_input:
-        student_ident = task_input['student']
-    return _update_problem_module_state(entry_id, course_id, problem_url, student_ident,
-                                        update_fcn, action_name, filter_fcn=filter_fcn,
-                                        xmodule_instance_args=xmodule_instance_args)
+    return update_problem_module_state(entry_id,
+                                       update_fcn, action_name, filter_fcn=filter_fcn,
+                                       xmodule_instance_args=xmodule_instance_args)
 
 
 @task
-def reset_problem_attempts(entry_id, course_id, task_input, xmodule_instance_args):
+def reset_problem_attempts(entry_id, xmodule_instance_args):
     """Resets problem attempts to zero for `problem_url` in `course_id` for all students.
 
     `entry_id` is the id value of the InstructorTask entry that corresponds to this task.
@@ -54,15 +50,14 @@ def reset_problem_attempts(entry_id, course_id, task_input, xmodule_instance_arg
     to instantiate an xmodule instance.
     """
     action_name = 'reset'
-    update_fcn = _reset_problem_attempts_module_state
-    problem_url = task_input.get('problem_url')
-    return _update_problem_module_state(entry_id, course_id, problem_url, None,
-                                        update_fcn, action_name, filter_fcn=None,
-                                        xmodule_instance_args=xmodule_instance_args)
+    update_fcn = reset_attempts_module_state
+    return update_problem_module_state(entry_id,
+                                       update_fcn, action_name, filter_fcn=None,
+                                       xmodule_instance_args=xmodule_instance_args)
 
 
 @task
-def delete_problem_state(entry_id, course_id, task_input, xmodule_instance_args):
+def delete_problem_state(entry_id, xmodule_instance_args):
     """Deletes problem state entirely for `problem_url` in `course_id` for all students.
 
     `entry_id` is the id value of the InstructorTask entry that corresponds to this task.
@@ -75,8 +70,7 @@ def delete_problem_state(entry_id, course_id, task_input, xmodule_instance_args)
     to instantiate an xmodule instance.
     """
     action_name = 'deleted'
-    update_fcn = _delete_problem_module_state
-    problem_url = task_input.get('problem_url')
-    return _update_problem_module_state(entry_id, course_id, problem_url, None,
-                                        update_fcn, action_name, filter_fcn=None,
-                                        xmodule_instance_args=xmodule_instance_args)
+    update_fcn = delete_problem_module_state
+    return update_problem_module_state(entry_id,
+                                       update_fcn, action_name, filter_fcn=None,
+                                       xmodule_instance_args=xmodule_instance_args)

lms/djangoapps/instructor_task/tasks_helper.py

 """
-This file contains tasks that are designed to perform background operations on the 
+This file contains tasks that are designed to perform background operations on the
 running state of a course.
 
 
@@ -50,7 +50,7 @@ class UpdateProblemModuleStateError(Exception):
 
 
 def _perform_module_state_update(course_id, module_state_key, student_identifier, update_fcn, action_name, filter_fcn,
-                                          xmodule_instance_args):
+                                 xmodule_instance_args):
     """
     Performs generic update by visiting StudentModule instances with the update_fcn provided.
 
@@ -161,7 +161,7 @@ def _save_course_task(course_task):
     course_task.save()
 
 
-def _update_problem_module_state(entry_id, course_id, module_state_key, student_ident, update_fcn, action_name, filter_fcn,
+def update_problem_module_state(entry_id, update_fcn, action_name, filter_fcn,
                                  xmodule_instance_args):
     """
     Performs generic update by visiting StudentModule instances with the update_fcn provided.
@@ -195,15 +195,20 @@ def _update_problem_module_state(entry_id, course_id, module_state_key, student_
     result object that Celery creates.
 
     """
-    task_id = current_task.request.id
-    fmt = 'Starting to update problem modules as task "{task_id}": course "{course_id}" problem "{state_key}": nothing {action} yet'
-    TASK_LOG.info(fmt.format(task_id=task_id, course_id=course_id, state_key=module_state_key, action=action_name))
 
     # get the InstructorTask to be updated.  If this fails, then let the exception return to Celery.
     # There's no point in catching it here.
     entry = InstructorTask.objects.get(pk=entry_id)
-    entry.task_id = task_id
-    _save_course_task(entry)
+
+    # get inputs to use in this task from the entry:
+    task_id = entry.task_id
+    course_id = entry.course_id
+    task_input = json.loads(entry.task_input)
+    module_state_key = task_input.get('problem_url')
+    student_ident = task_input['student'] if 'student' in task_input else None
+
+    fmt = 'Starting to update problem modules as task "{task_id}": course "{course_id}" problem "{state_key}": nothing {action} yet'
+    TASK_LOG.info(fmt.format(task_id=task_id, course_id=course_id, state_key=module_state_key, action=action_name))
 
     # add task_id to xmodule_instance_args, so that it can be output with tracking info:
     if xmodule_instance_args is not None:
@@ -212,6 +217,16 @@ def _update_problem_module_state(entry_id, course_id, module_state_key, student_
     # now that we have an entry we can try to catch failures:
     task_progress = None
     try:
+        # check that the task_id submitted in the InstructorTask matches the current task
+        # that is running.
+        request_task_id = current_task.request.id
+        if task_id != request_task_id:
+            fmt = 'Requested task "{task_id}" did not match actual task "{actual_id}"'
+            message = fmt.format(task_id=task_id, course_id=course_id, state_key=module_state_key, actual_id=request_task_id)
+            TASK_LOG.error(message)
+            raise UpdateProblemModuleStateError(message)
+
+        # now do the work:
         with dog_stats_api.timer('courseware.tasks.module.{0}.overall_time'.format(action_name)):
             task_progress = _perform_module_state_update(course_id, module_state_key, student_ident, update_fcn,
                                                          action_name, filter_fcn, xmodule_instance_args)
@@ -280,7 +295,7 @@ def _get_module_instance_for_task(course_id, student, module_descriptor, xmodule
 
 
 @transaction.autocommit
-def _rescore_problem_module_state(module_descriptor, student_module, xmodule_instance_args=None):
+def rescore_problem_module_state(module_descriptor, student_module, xmodule_instance_args=None):
     '''
     Takes an XModule descriptor and a corresponding StudentModule object, and
     performs rescoring on the student's problem submission.
@@ -330,7 +345,7 @@ def _rescore_problem_module_state(module_descriptor, student_module, xmodule_ins
 
 
 @transaction.autocommit
-def _reset_problem_attempts_module_state(_module_descriptor, student_module, xmodule_instance_args=None):
+def reset_attempts_module_state(_module_descriptor, student_module, xmodule_instance_args=None):
     """
     Resets problem attempts to zero for specified `student_module`.
 
@@ -356,7 +371,7 @@ def _reset_problem_attempts_module_state(_module_descriptor, student_module, xmo
 
 
 @transaction.autocommit
-def _delete_problem_module_state(_module_descriptor, student_module, xmodule_instance_args=None):
+def delete_problem_module_state(_module_descriptor, student_module, xmodule_instance_args=None):
     """
     Delete the StudentModule entry.
 

lms/djangoapps/instructor_task/tests/test_api.py

@@ -24,7 +24,7 @@ from instructor_task.api import (get_running_instructor_tasks,
                                  submit_delete_problem_state_for_all_students)
 
 from instructor_task.api_helper import (QUEUING,
-                                        AlreadyRunningError,
+#                                        AlreadyRunningError,
                                         encode_problem_and_student_input,
                                         )
 
@@ -61,12 +61,12 @@ class TaskSubmitTestCase(TestCase):
         task_input, task_key = encode_problem_and_student_input(self.problem_url, student)
 
         instructor_task = InstructorTaskFactory.create(course_id=TEST_COURSE_ID,
-                                               requester=self.instructor,
-                                               task_input=json.dumps(task_input),
-                                               task_key=task_key,
-                                               task_id=task_id,
-                                               task_state=task_state,
-                                               task_output=progress_json)
+                                                       requester=self.instructor,
+                                                       task_input=json.dumps(task_input),
+                                                       task_key=task_key,
+                                                       task_id=task_id,
+                                                       task_state=task_state,
+                                                       task_output=progress_json)
         return instructor_task
 
     def _create_failure_entry(self):
@@ -101,6 +101,7 @@ class TaskSubmitTestCase(TestCase):
         self.assertEquals(set(task_ids), set(progress_task_ids))
 
     def _get_instructor_task_status(self, task_id):
+        """Returns status corresponding to task_id via api method."""
         request = Mock()
         request.REQUEST = {'task_id': task_id}
         return instructor_task_status(request)

lms/djangoapps/instructor_task/tests/test_integration.py

@@ -2,7 +2,7 @@
 Integration Test for LMS instructor-initiated background tasks
 
 Runs tasks on answers to course problems to validate that code
-paths actually work. 
+paths actually work.
 
 """
 import logging
@@ -32,7 +32,6 @@ from instructor_task.api import (submit_rescore_problem_for_all_students,
                                  submit_reset_problem_attempts_for_all_students,
                                  submit_delete_problem_state_for_all_students)
 from instructor_task.models import InstructorTask
-from instructor_task.tests.factories import InstructorTaskFactory
 from instructor_task.views import instructor_task_status
 
 
@@ -235,6 +234,7 @@ class TestRescoringBase(LoginEnrollmentTestCase, ModuleStoreTestCase):
             self.assertGreater(len(state['student_answers']), 0)
 
     def get_task_status(self, task_id):
+        """Use api method to fetch task status, using mock request."""
         mock_request = Mock()
         mock_request.REQUEST = {'task_id': task_id}
         response = instructor_task_status(mock_request)