Skip to content

Commit

Permalink
RED-2483: graceful error handling for migrate_transcripts command
Browse files Browse the repository at this point in the history
 - a course will either be successful or will fail in one atomic SQL
   transaction.
 - a course failure will be logged, then continue for next course
 - ItemNotFoundError will be ignored seperately from other errors since
   it's usually means that we have some other problem that
   the `migrate_transcripts` command shouldn't worry about
  • Loading branch information
OmarIthawi committed Mar 1, 2022
1 parent 6798d80 commit d83e6c4
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.video_module import VideoBlock
from xmodule.video_module.transcripts_utils import save_to_store
from xmodule.modulestore.exceptions import ItemNotFoundError


LOGGER_NAME = "cms.djangoapps.contentstore.tasks"

Expand Down Expand Up @@ -311,6 +313,39 @@ def test_migrate_transcripts_exception_logging(self):
*expected_log
)

@ddt.unpack
@ddt.data({
'exception_class': ItemNotFoundError,
'log_message': 'course-transcript-migration-failed-with-not-found-error-exc',
'log_type': 'INFO',
}, {
'exception_class': ValueError, # Some random exception
'log_message': 'course-transcript-migration-failed-with-unknown-exc',
'log_type': 'ERROR',
})
@patch('cms.djangoapps.contentstore.tasks.async_migrate_transcript')
def test_tahoe_migrate_additional_error_handling(self, mock_migrate, exception_class, log_message, log_type):
"""
Tahoe: Test migrate transcripts custom exception handling for ItemNotFoundError and other errors.
"""
exception = exception_class('Test exception')
mock_migrate.side_effect = exception
course_id = six.text_type(self.course_2.id)
expected_log = (
(
'cms.djangoapps.contentstore.tasks', log_type,
(u'[Transcript Migration] [run=1] [{}] [course={}] [exc={}]'.format(
log_message, course_id, str(exception)
))
),
)

with LogCapture(LOGGER_NAME, level=logging.INFO) as logger:
call_command('migrate_transcripts', '--course-id', six.text_type(self.course_2.id), '--commit')
logger.check(
*expected_log
)

@ddt.data(*itertools.product([1, 2], [True, False], [True, False]))
@ddt.unpack
@patch('contentstore.management.commands.migrate_transcripts.log')
Expand Down
19 changes: 18 additions & 1 deletion cms/djangoapps/contentstore/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,25 @@ def enqueue_async_migrate_transcripts_tasks(course_keys,
'command_run': command_run
}

# TODO: Undo all the migrate_transcripts edits: https://github.com/appsembler/edx-platform/issues/1068
# temp import, kept here to reduce migration conflicts
from django.db import transaction
from xmodule.modulestore.exceptions import ItemNotFoundError

for course_key in course_keys:
async_migrate_transcript(text_type(course_key), **kwargs)
try:
with transaction.atomic():
async_migrate_transcript(text_type(course_key), **kwargs)
except ItemNotFoundError as not_found_exc:
LOGGER.info(
'[%s] [run=%s] [course-transcript-migration-failed-with-not-found-error-exc] [course=%s] [exc=%s]',
MIGRATION_LOGS_PREFIX, command_run, course_key, str(not_found_exc)
)
except Exception as exc:
LOGGER.exception(
'[%s] [run=%s] [course-transcript-migration-failed-with-unknown-exc] [course=%s] [exc=%s]',
MIGRATION_LOGS_PREFIX, command_run, course_key, str(exc)
)


def get_course_videos(course_key):
Expand Down

0 comments on commit d83e6c4

Please sign in to comment.