diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index d24eb07d9d..79f1534f41 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -263,50 +263,3 @@ class OfflineComputedGradeLog(models.Model): def __unicode__(self): return "[OCGLog] %s: %s" % (self.course_id, self.created) - - -class CourseTask(models.Model): - """ - Stores information about background tasks that have been submitted to - perform course-specific work. - Examples include grading and rescoring. - - `task_type` identifies the kind of task being performed, e.g. rescoring. - `course_id` uses the course run's unique id to identify the course. - `task_input` stores input arguments as JSON-serialized dict, for reporting purposes. - Examples include url of problem being rescored, id of student if only one student being rescored. - `task_key` stores relevant input arguments encoded into key value for testing to see - if the task is already running (together with task_type and course_id). - - `task_id` stores the id used by celery for the background task. - `task_state` stores the last known state of the celery task - `task_output` stores the output of the celery task. - Format is a JSON-serialized dict. Content varies by task_type and task_state. - - `requester` stores id of user who submitted the task - `created` stores date that entry was first created - `updated` stores date that entry was last modified - """ - task_type = models.CharField(max_length=50, db_index=True) - course_id = models.CharField(max_length=255, db_index=True) - task_key = models.CharField(max_length=255, db_index=True) - task_input = models.CharField(max_length=255) - task_id = models.CharField(max_length=255, db_index=True) # max_length from celery_taskmeta - task_state = models.CharField(max_length=50, null=True, db_index=True) # max_length from celery_taskmeta - task_output = models.CharField(max_length=1024, null=True) - requester = models.ForeignKey(User, db_index=True) - created = models.DateTimeField(auto_now_add=True, null=True) - updated = models.DateTimeField(auto_now=True) - - def __repr__(self): - return 'CourseTask<%r>' % ({ - 'task_type': self.task_type, - 'course_id': self.course_id, - 'task_input': self.task_input, - 'task_id': self.task_id, - 'task_state': self.task_state, - 'task_output': self.task_output, - },) - - def __unicode__(self): - return unicode(repr(self)) diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index a79e574937..d2a8b78887 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -54,6 +54,14 @@ def submit_rescore_problem_for_student(request, course_id, problem_url, student) ItemNotFoundException is raised if the problem doesn't exist, or AlreadyRunningError if the problem is already being rescored for this student, or NotImplementedError if the problem doesn't support rescoring. + + This method makes sure the InstructorTask entry is committed. + When called from any view that is wrapped by TransactionMiddleware, + and thus in a "commit-on-success" transaction, an autocommit buried within here + will cause any pending transaction to be committed by a successful + save here. Any future database operations will take place in a + separate transaction. + """ # check arguments: let exceptions return up to the caller. check_arguments_for_rescoring(course_id, problem_url) @@ -76,6 +84,13 @@ def submit_rescore_problem_for_all_students(request, course_id, problem_url): ItemNotFoundException is raised if the problem doesn't exist, or AlreadyRunningError if the problem is already being rescored, or NotImplementedError if the problem doesn't support rescoring. + + This method makes sure the InstructorTask entry is committed. + When called from any view that is wrapped by TransactionMiddleware, + and thus in a "commit-on-success" transaction, an autocommit buried within here + will cause any pending transaction to be committed by a successful + save here. Any future database operations will take place in a + separate transaction. """ # check arguments: let exceptions return up to the caller. check_arguments_for_rescoring(course_id, problem_url) @@ -98,6 +113,13 @@ def submit_reset_problem_attempts_for_all_students(request, course_id, problem_u ItemNotFoundException is raised if the problem doesn't exist, or AlreadyRunningError if the problem is already being reset. + + This method makes sure the InstructorTask entry is committed. + When called from any view that is wrapped by TransactionMiddleware, + and thus in a "commit-on-success" transaction, an autocommit buried within here + will cause any pending transaction to be committed by a successful + save here. Any future database operations will take place in a + separate transaction. """ # check arguments: make sure that the problem_url is defined # (since that's currently typed in). If the corresponding module descriptor doesn't exist, @@ -121,6 +143,13 @@ def submit_delete_problem_state_for_all_students(request, course_id, problem_url ItemNotFoundException is raised if the problem doesn't exist, or AlreadyRunningError if the particular problem is already being deleted. + + This method makes sure the InstructorTask entry is committed. + When called from any view that is wrapped by TransactionMiddleware, + and thus in a "commit-on-success" transaction, an autocommit buried within here + will cause any pending transaction to be committed by a successful + save here. Any future database operations will take place in a + separate transaction. """ # check arguments: make sure that the problem_url is defined # (since that's currently typed in). If the corresponding module descriptor doesn't exist, diff --git a/lms/djangoapps/instructor_task/api_helper.py b/lms/djangoapps/instructor_task/api_helper.py index 13bb9af87c..3decd36e1f 100644 --- a/lms/djangoapps/instructor_task/api_helper.py +++ b/lms/djangoapps/instructor_task/api_helper.py @@ -1,7 +1,8 @@ import hashlib import json import logging -# from django.http import HttpResponse +from uuid import uuid4 + from django.db import transaction from celery.result import AsyncResult @@ -11,7 +12,6 @@ from courseware.module_render import get_xqueue_callback_url_prefix from xmodule.modulestore.django import modulestore from instructor_task.models import InstructorTask -# from instructor_task.views import get_task_completion_info from instructor_task.tasks_helper import PROGRESS @@ -40,16 +40,27 @@ def _reserve_task(course_id, task_type, task_key, task_input, requester): Creates a database entry to indicate that a task is in progress. Throws AlreadyRunningError if the task is already in progress. + Includes the creation of an arbitrary value for task_id, to be + submitted with the task call to celery. Autocommit annotation makes sure the database entry is committed. + When called from any view that is wrapped by TransactionMiddleware, + and thus in a "commit-on-success" transaction, this autocommit here + will cause any pending transaction to be committed by a successful + save here. Any future database operations will take place in a + separate transaction. """ if _task_is_running(course_id, task_type, task_key): raise AlreadyRunningError("requested task is already running") - # Create log entry now, so that future requests won't: no task_id yet.... + # create the task_id here, and pass it into celery: + task_id = str(uuid4()) + + # Create log entry now, so that future requests won't tasklog_args = {'course_id': course_id, 'task_type': task_type, + 'task_id': task_id, 'task_key': task_key, 'task_input': json.dumps(task_input), 'task_state': 'QUEUING', @@ -59,21 +70,6 @@ def _reserve_task(course_id, task_type, task_key, task_input, requester): return instructor_task -@transaction.autocommit -def _update_task(instructor_task, task_result): - """ - Updates a database entry with information about the submitted task. - - Autocommit annotation makes sure the database entry is committed. - """ - # we at least update the entry with the task_id, and for ALWAYS_EAGER mode, - # we update other status as well. (For non-ALWAYS_EAGER modes, the entry - # should not have changed except for setting PENDING state and the - # addition of the task_id.) - _update_instructor_task(instructor_task, task_result) - instructor_task.save() - - def _get_xmodule_instance_args(request): """ Calculate parameters needed for instantiating xmodule instances. @@ -98,8 +94,7 @@ def _update_instructor_task(instructor_task, task_result): """ Updates and possibly saves a InstructorTask entry based on a task Result. - Used when a task initially returns, as well as when updated status is - requested. + Used when updated status is requested. The `instructor_task` that is passed in is updated in-place, but is usually not saved. In general, tasks that have finished (either with @@ -110,25 +105,11 @@ def _update_instructor_task(instructor_task, task_result): opportunity to update the InstructorTask entry. Calculates json to store in "task_output" field of the `instructor_task`, - as well as updating the task_state and task_id (which may not yet be set - if this is the first call after the task is submitted). - -TODO: Update -- no longer return anything, or maybe the resulting instructor_task. - - Returns a dict, with the following keys: - 'message': status message reporting on progress, or providing exception message if failed. - 'task_progress': dict containing progress information. This includes: - 'attempted': number of attempts made - 'updated': number of attempts that "succeeded" - 'total': number of possible subtasks to attempt - 'action_name': user-visible verb to use in status messages. Should be past-tense. - 'duration_ms': how long the task has (or had) been running. - 'task_traceback': optional, returned if task failed and produced a traceback. - 'succeeded': on complete tasks, 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. + as well as updating the task_state. + For a successful task, the json contains the output of the task result. + For a failed task, the json contains "exception", "message", and "traceback" + keys. A revoked task just has a "message" stating it was revoked. """ # Pull values out of the result object as close to each other as possible. # If we wait and check the values later, the values for the state and result @@ -141,59 +122,49 @@ TODO: Update -- no longer return anything, or maybe the resulting instructor_tas # Assume we don't always update the InstructorTask entry if we don't have to: entry_needs_saving = False - output = {} + task_progress = None if result_state in [PROGRESS, SUCCESS]: # construct a status message directly from the task result's result: # it needs to go back with the entry passed in. - instructor_task.task_output = json.dumps(returned_result) -# output['task_progress'] = returned_result - log.info("background task (%s), succeeded: %s", task_id, returned_result) - + log.info("background task (%s), state %s: result: %s", task_id, result_state, returned_result) + task_progress = returned_result elif result_state == FAILURE: # on failure, the result's result contains the exception that caused the failure exception = returned_result traceback = result_traceback if result_traceback is not None else '' task_progress = {'exception': type(exception).__name__, 'message': str(exception.message)} -# output['message'] = exception.message log.warning("background task (%s) failed: %s %s", task_id, returned_result, traceback) if result_traceback is not None: -# output['task_traceback'] = result_traceback # truncate any traceback that goes into the InstructorTask model: task_progress['traceback'] = result_traceback[:700] - # save progress into the entry, even if it's not being saved: - # when celery is run in "ALWAYS_EAGER" mode, progress needs to go back - # with the entry passed in. - instructor_task.task_output = json.dumps(task_progress) -# output['task_progress'] = task_progress elif result_state == REVOKED: # on revocation, the result's result doesn't contain anything # but we cannot rely on the worker thread to set this status, # so we set it here. entry_needs_saving = True - message = 'Task revoked before running' -# output['message'] = message log.warning("background task (%s) revoked.", task_id) - task_progress = {'message': message} - instructor_task.task_output = json.dumps(task_progress) -# output['task_progress'] = task_progress + task_progress = {'message': 'Task revoked before running'} - # Always update the local version of the entry if the state has changed. - # This is important for getting the task_id into the initial version - # of the instructor_task, and also for development environments - # when this code is executed when celery is run in "ALWAYS_EAGER" mode. - if result_state != instructor_task.task_state: - instructor_task.task_state = result_state - instructor_task.task_id = task_id + # save progress and state into the entry, even if it's not being saved: + # when celery is run in "ALWAYS_EAGER" mode, progress needs to go back + # with the entry passed in. + instructor_task.task_state = result_state + if task_progress is not None: + instructor_task.task_output = json.dumps(task_progress) if entry_needs_saving: instructor_task.save() - return output +def get_updated_instructor_task(task_id): + """ + Returns InstructorTask object corresponding to a given `task_id`. -def _get_updated_instructor_task(task_id): + If the InstructorTask thinks the task is still running, then + the task's result is checked to return an updated state and output. + """ # First check if the task_id is known try: instructor_task = InstructorTask.objects.get(task_id=task_id) @@ -210,49 +181,31 @@ def _get_updated_instructor_task(task_id): return instructor_task -# def _get_instructor_task_status(task_id): -def _get_instructor_task_status(instructor_task): +def get_status_from_instructor_task(instructor_task): """ - Get the status for a given task_id. + Get the status for a given InstructorTask entry. Returns a dict, with the following keys: - 'task_id' - 'task_state' - 'in_progress': boolean indicating if the task is still running. - 'message': status message reporting on progress, or providing exception message if failed. + 'task_id': id assigned by LMS and used by celery. + 'task_state': state of task as stored in celery's result store. + 'in_progress': boolean indicating if task is still running. 'task_progress': dict containing progress information. This includes: 'attempted': number of attempts made 'updated': number of attempts that "succeeded" 'total': number of possible subtasks to attempt 'action_name': user-visible verb to use in status messages. Should be past-tense. 'duration_ms': how long the task has (or had) been running. - 'task_traceback': optional, returned if task failed and produced a traceback. - 'succeeded': on complete tasks, 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. + 'exception': name of exception class raised in failed tasks. + 'message': returned for failed and revoked tasks. + 'traceback': optional, returned if task failed and produced a traceback. If task doesn't exist, returns None. - If task has been REVOKED, the InstructorTask entry will be updated. + If task has been REVOKED, the InstructorTask entry will be updated in + persistent storage as a side effect. """ -# # First check if the task_id is known -# try: -# instructor_task = InstructorTask.objects.get(task_id=task_id) -# except InstructorTask.DoesNotExist: -# log.warning("query for InstructorTask status failed: task_id=(%s) not found", task_id) -# return None - status = {} - # if the task is not already known to be done, then we need to query - # the underlying task's result object: -# if instructor_task.task_state not in READY_STATES: -# result = AsyncResult(task_id) -# status.update(_update_instructor_task(instructor_task, result)) - -# elif instructor_task.task_output is not None: - # task is already known to have finished, but report on its status: if instructor_task.task_output is not None: status['task_progress'] = json.loads(instructor_task.task_output) @@ -261,11 +214,6 @@ def _get_instructor_task_status(instructor_task): status['task_state'] = instructor_task.task_state status['in_progress'] = instructor_task.task_state not in READY_STATES -# if instructor_task.task_state in READY_STATES: -# succeeded, message = get_task_completion_info(instructor_task) -# status['message'] = message -# status['succeeded'] = succeeded - return status @@ -312,19 +260,24 @@ def submit_task(request, task_type, task_class, course_id, task_input, task_key) checking to see if the task is already running. The `task_input` is also passed so that it can be stored in the resulting InstructorTask entry. Arguments are extracted from the `request` provided by the originating server request. Then the task is submitted to run - asynchronously, using the specified `task_class`. Finally the InstructorTask entry is - updated in order to store the task_id. + asynchronously, using the specified `task_class` and using the task_id constructed for it. `AlreadyRunningError` is raised if the task is already running. + + The _reserve_task method makes sure the InstructorTask entry is committed. + When called from any view that is wrapped by TransactionMiddleware, + and thus in a "commit-on-success" transaction, an autocommit buried within here + will cause any pending transaction to be committed by a successful + save here. Any future database operations will take place in a + separate transaction. + """ # check to see if task is already running, and reserve it otherwise: instructor_task = _reserve_task(course_id, task_type, task_key, task_input, request.user) # submit task: + task_id = instructor_task.task_id task_args = [instructor_task.id, course_id, task_input, _get_xmodule_instance_args(request)] - task_result = task_class.apply_async(task_args) - - # Update info in table with the resulting task_id (and state). - _update_task(instructor_task, task_result) + task_class.apply_async(task_args, task_id=task_id) return instructor_task diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index 80a1701cc2..14af159cd3 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -60,14 +60,14 @@ class TaskSubmitTestCase(TestCase): progress_json = json.dumps(task_output) task_input, task_key = encode_problem_and_student_input(self.problem_url, student) - course_task = InstructorTaskFactory.create(course_id=TEST_COURSE_ID, + 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) - return course_task + return instructor_task def _create_failure_entry(self): """Creates a InstructorTask entry representing a failed task.""" @@ -97,24 +97,24 @@ class TaskSubmitTestCase(TestCase): self._create_failure_entry() self._create_success_entry() progress_task_ids = [self._create_progress_entry().task_id for _ in range(1, 5)] - task_ids = [course_task.task_id for course_task in get_running_instructor_tasks(TEST_COURSE_ID)] + task_ids = [instructor_task.task_id for instructor_task in get_running_instructor_tasks(TEST_COURSE_ID)] self.assertEquals(set(task_ids), set(progress_task_ids)) - def _get_course_task_status(self, task_id): + def _get_instructor_task_status(self, task_id): request = Mock() request.REQUEST = {'task_id': task_id} return instructor_task_status(request) - def test_course_task_status(self): - course_task = self._create_failure_entry() - task_id = course_task.task_id + def test_instructor_task_status(self): + instructor_task = self._create_failure_entry() + task_id = instructor_task.task_id request = Mock() request.REQUEST = {'task_id': task_id} response = instructor_task_status(request) output = json.loads(response.content) self.assertEquals(output['task_id'], task_id) - def test_course_task_status_list(self): + def test_instructor_task_status_list(self): # Fetch status for existing tasks by arg list, as if called from ajax. # Note that ajax does something funny with the marshalling of # list data, so the key value has "[]" appended to it. @@ -128,9 +128,9 @@ class TaskSubmitTestCase(TestCase): self.assertEquals(output[task_id]['task_id'], task_id) def test_get_status_from_failure(self): - course_task = self._create_failure_entry() - task_id = course_task.task_id - response = self._get_course_task_status(task_id) + instructor_task = self._create_failure_entry() + task_id = instructor_task.task_id + response = self._get_instructor_task_status(task_id) output = json.loads(response.content) self.assertEquals(output['task_id'], task_id) self.assertEquals(output['task_state'], FAILURE) @@ -138,9 +138,9 @@ class TaskSubmitTestCase(TestCase): self.assertEquals(output['message'], TEST_FAILURE_MESSAGE) def test_get_status_from_success(self): - course_task = self._create_success_entry() - task_id = course_task.task_id - response = self._get_course_task_status(task_id) + instructor_task = self._create_success_entry() + task_id = instructor_task.task_id + response = self._get_instructor_task_status(task_id) output = json.loads(response.content) self.assertEquals(output['task_id'], task_id) self.assertEquals(output['task_state'], SUCCESS) @@ -148,8 +148,8 @@ class TaskSubmitTestCase(TestCase): def test_update_progress_to_progress(self): # view task entry for task in progress - course_task = self._create_progress_entry() - task_id = course_task.task_id + instructor_task = self._create_progress_entry() + task_id = instructor_task.task_id mock_result = Mock() mock_result.task_id = task_id mock_result.state = PROGRESS @@ -159,7 +159,7 @@ class TaskSubmitTestCase(TestCase): 'action_name': 'rescored'} with patch('celery.result.AsyncResult.__new__') as mock_result_ctor: mock_result_ctor.return_value = mock_result - response = self._get_course_task_status(task_id) + response = self._get_instructor_task_status(task_id) output = json.loads(response.content) self.assertEquals(output['task_id'], task_id) self.assertEquals(output['task_state'], PROGRESS) @@ -168,8 +168,8 @@ class TaskSubmitTestCase(TestCase): def test_update_progress_to_failure(self): # view task entry for task in progress that later fails - course_task = self._create_progress_entry() - task_id = course_task.task_id + instructor_task = self._create_progress_entry() + task_id = instructor_task.task_id mock_result = Mock() mock_result.task_id = task_id mock_result.state = FAILURE @@ -177,7 +177,7 @@ class TaskSubmitTestCase(TestCase): mock_result.traceback = "random traceback" with patch('celery.result.AsyncResult.__new__') as mock_result_ctor: mock_result_ctor.return_value = mock_result - response = self._get_course_task_status(task_id) + response = self._get_instructor_task_status(task_id) output = json.loads(response.content) self.assertEquals(output['task_id'], task_id) self.assertEquals(output['task_state'], FAILURE) @@ -186,14 +186,14 @@ class TaskSubmitTestCase(TestCase): def test_update_progress_to_revoked(self): # view task entry for task in progress that later fails - course_task = self._create_progress_entry() - task_id = course_task.task_id + instructor_task = self._create_progress_entry() + task_id = instructor_task.task_id mock_result = Mock() mock_result.task_id = task_id mock_result.state = REVOKED with patch('celery.result.AsyncResult.__new__') as mock_result_ctor: mock_result_ctor.return_value = mock_result - response = self._get_course_task_status(task_id) + response = self._get_instructor_task_status(task_id) output = json.loads(response.content) self.assertEquals(output['task_id'], task_id) self.assertEquals(output['task_state'], REVOKED) @@ -201,10 +201,10 @@ class TaskSubmitTestCase(TestCase): self.assertEquals(output['message'], "Task revoked before running") def _get_output_for_task_success(self, attempted, updated, total, student=None): - """returns the task_id and the result returned by course_task_status().""" + """returns the task_id and the result returned by instructor_task_status().""" # view task entry for task in progress - course_task = self._create_progress_entry(student) - task_id = course_task.task_id + instructor_task = self._create_progress_entry(student) + task_id = instructor_task.task_id mock_result = Mock() mock_result.task_id = task_id mock_result.state = SUCCESS @@ -214,7 +214,7 @@ class TaskSubmitTestCase(TestCase): 'action_name': 'rescored'} with patch('celery.result.AsyncResult.__new__') as mock_result_ctor: mock_result_ctor.return_value = mock_result - response = self._get_course_task_status(task_id) + response = self._get_instructor_task_status(task_id) output = json.loads(response.content) return task_id, output @@ -271,9 +271,9 @@ class TaskSubmitTestCase(TestCase): # def test_submit_when_running(self): # # get exception when trying to submit a task that is already running -# course_task = self._create_progress_entry() -# problem_url = json.loads(course_task.task_input).get('problem_url') -# course_id = course_task.course_id +# instructor_task = self._create_progress_entry() +# problem_url = json.loads(instructor_task.task_input).get('problem_url') +# course_id = instructor_task.course_id # # requester doesn't have to be the same when determining if a task is already running # request = Mock() # request.user = self.instructor diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index 704cc265a8..e875f5d8e7 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -9,7 +9,6 @@ import logging import json from mock import Mock, patch import textwrap -from uuid import uuid4 from celery.states import SUCCESS, FAILURE from django.contrib.auth.models import User @@ -23,17 +22,18 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.exceptions import ItemNotFoundError -from student.tests.factories import CourseEnrollmentFactory, UserFactory, AdminFactory +from student.tests.factories import CourseEnrollmentFactory, UserFactory, AdminFactory from courseware.model_data import StudentModule +from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_MONGO_MODULESTORE + from instructor_task.api import (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) -from instructor_task.views import instructor_task_status - -from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_MONGO_MODULESTORE +from instructor_task.models import InstructorTask from instructor_task.tests.factories import InstructorTaskFactory +from instructor_task.views import instructor_task_status log = logging.getLogger(__name__) @@ -306,6 +306,7 @@ class TestRescoring(TestRescoringBase): instructor_task = self.submit_rescore_all_student_answers('instructor', problem_url_name) # check instructor_task returned + instructor_task = InstructorTask.objects.get(id=instructor_task.id) self.assertEqual(instructor_task.task_state, 'FAILURE') self.assertEqual(instructor_task.requester.username, 'instructor') self.assertEqual(instructor_task.task_type, 'rescore_problem') @@ -359,6 +360,8 @@ class TestRescoring(TestRescoringBase): self.submit_student_answer('u1', problem_url_name, ["answer1", "answer2"]) instructor_task = self.submit_rescore_all_student_answers('instructor', problem_url_name) + + instructor_task = InstructorTask.objects.get(id=instructor_task.id) self.assertEqual(instructor_task.task_state, FAILURE) status = json.loads(instructor_task.task_output) self.assertEqual(status['exception'], 'NotImplementedError') @@ -510,7 +513,8 @@ class TestResetAttempts(TestRescoringBase): mock_save.side_effect = ZeroDivisionError(expected_message) instructor_task = self.reset_problem_attempts('instructor', problem_url_name) - # check instructor_task returned + # check instructor_task + instructor_task = InstructorTask.objects.get(id=instructor_task.id) self.assertEqual(instructor_task.task_state, FAILURE) self.assertEqual(instructor_task.requester.username, 'instructor') self.assertEqual(instructor_task.task_type, 'reset_problem_attempts') @@ -529,6 +533,7 @@ class TestResetAttempts(TestRescoringBase): """confirm that a non-problem can still be successfully reset""" problem_url_name = self.problem_section.location.url() instructor_task = self.reset_problem_attempts('instructor', problem_url_name) + instructor_task = InstructorTask.objects.get(id=instructor_task.id) self.assertEqual(instructor_task.task_state, SUCCESS) def test_reset_nonexistent_problem(self): @@ -586,6 +591,7 @@ class TestDeleteProblem(TestRescoringBase): instructor_task = self.delete_problem_state('instructor', problem_url_name) # check instructor_task returned + instructor_task = InstructorTask.objects.get(id=instructor_task.id) self.assertEqual(instructor_task.task_state, FAILURE) self.assertEqual(instructor_task.requester.username, 'instructor') self.assertEqual(instructor_task.task_type, 'delete_problem_state') @@ -604,6 +610,7 @@ class TestDeleteProblem(TestRescoringBase): """confirm that a non-problem can still be successfully deleted""" problem_url_name = self.problem_section.location.url() instructor_task = self.delete_problem_state('instructor', problem_url_name) + instructor_task = InstructorTask.objects.get(id=instructor_task.id) self.assertEqual(instructor_task.task_state, SUCCESS) def test_delete_nonexistent_module(self): diff --git a/lms/djangoapps/instructor_task/views.py b/lms/djangoapps/instructor_task/views.py index 5af0d46d46..c5970645ff 100644 --- a/lms/djangoapps/instructor_task/views.py +++ b/lms/djangoapps/instructor_task/views.py @@ -6,8 +6,8 @@ from django.http import HttpResponse from celery.states import FAILURE, REVOKED, READY_STATES -from instructor_task.api_helper import (_get_instructor_task_status, - _get_updated_instructor_task) +from instructor_task.api_helper import (get_status_from_instructor_task, + get_updated_instructor_task) log = logging.getLogger(__name__) @@ -31,10 +31,36 @@ 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': status message reporting on 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. + 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. + 'task_state': state of task as stored in celery's result store. + 'in_progress': boolean indicating if task is still running. + 'task_progress': dict containing progress information. This includes: + 'attempted': number of attempts made + 'updated': number of attempts that "succeeded" + 'total': number of possible subtasks to attempt + 'action_name': user-visible verb to use in status messages. Should be past-tense. + 'duration_ms': how long the task has (or had) been running. + 'exception': name of exception class raised in failed tasks. + 'message': returned for failed and revoked tasks. + 'traceback': optional, returned if task failed and produced a traceback. + """ def get_instructor_task_status(task_id): - instructor_task = _get_updated_instructor_task(task_id) - status = _get_instructor_task_status(instructor_task) + """ + Returns status for a specific task. + + Written as an internal method here (rather than as a helper) + so that get_task_completion_info() can be called without + causing a circular dependency (since it's also called directly). + """ + instructor_task = get_updated_instructor_task(task_id) + status = get_status_from_instructor_task(instructor_task) if instructor_task.task_state in READY_STATES: succeeded, message = get_task_completion_info(instructor_task) status['message'] = message