From c7c2f808e718101c7e2292a7aa265373b2957d35 Mon Sep 17 00:00:00 2001 From: Qubad786 Date: Mon, 25 Jun 2018 18:41:54 +0500 Subject: [PATCH] Add a field to transcript migration data model to track number of runs include command run tracking information in logging --- .../commands/migrate_transcripts.py | 5 +- .../tests/test_migrate_transcripts.py | 16 ++--- cms/djangoapps/contentstore/tasks.py | 70 +++++++++++-------- ..._transcriptmigrationsetting_command_run.py | 20 ++++++ .../core/djangoapps/video_config/models.py | 11 ++- 5 files changed, 83 insertions(+), 39 deletions(-) create mode 100644 openedx/core/djangoapps/video_config/migrations/0004_transcriptmigrationsetting_command_run.py diff --git a/cms/djangoapps/contentstore/management/commands/migrate_transcripts.py b/cms/djangoapps/contentstore/management/commands/migrate_transcripts.py index c17cea260b..b8f89e212f 100644 --- a/cms/djangoapps/contentstore/management/commands/migrate_transcripts.py +++ b/cms/djangoapps/contentstore/management/commands/migrate_transcripts.py @@ -113,4 +113,7 @@ class Command(BaseCommand): Invokes the migrate transcripts enqueue function. """ course_keys, force_update, commit = self._get_migration_options(options) - enqueue_async_migrate_transcripts_tasks(course_keys, force_update=force_update, commit=commit) + command_run = self._latest_settings().increment_run() if commit else -1 + enqueue_async_migrate_transcripts_tasks( + course_keys=course_keys, commit=commit, command_run=command_run, force_update=force_update + ) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_migrate_transcripts.py b/cms/djangoapps/contentstore/management/commands/tests/test_migrate_transcripts.py index 126c65831a..5494d2cdb9 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_migrate_transcripts.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_migrate_transcripts.py @@ -221,24 +221,24 @@ class TestMigrateTranscripts(ModuleStoreTestCase): expected_log = ( ( 'cms.djangoapps.contentstore.tasks', 'INFO', - (u'[Transcript Migration] [video-transcripts-migration-process-started-for-course] ' + (u'[Transcript Migration] [run=-1] [video-transcripts-migration-process-started-for-course] ' u'[course={}]'.format(course_id)) ), ( 'cms.djangoapps.contentstore.tasks', 'INFO', - (u'[Transcript Migration] [video-transcript-will-be-migrated] ' + (u'[Transcript Migration] [run=-1] [video-transcript-will-be-migrated] ' u'[revision=rev-opt-published-only] [video={}] [edx_video_id=test_edx_video_id] ' u'[language_code=hr]'.format(self.video_descriptor.location)) ), ( 'cms.djangoapps.contentstore.tasks', 'INFO', - (u'[Transcript Migration] [video-transcript-will-be-migrated] ' + (u'[Transcript Migration] [run=-1] [video-transcript-will-be-migrated] ' u'[revision=rev-opt-published-only] [video={}] [edx_video_id=test_edx_video_id] ' u'[language_code=ge]'.format(self.video_descriptor.location)) ), ( 'cms.djangoapps.contentstore.tasks', 'INFO', - (u'[Transcript Migration] [transcripts-migration-tasks-submitted] ' + (u'[Transcript Migration] [run=-1] [transcripts-migration-tasks-submitted] ' u'[transcripts_count=2] [course={}] ' u'[revision=rev-opt-published-only] [video={}]'.format(course_id, self.video_descriptor.location)) ) @@ -258,24 +258,24 @@ class TestMigrateTranscripts(ModuleStoreTestCase): expected_log = ( ( 'cms.djangoapps.contentstore.tasks', 'INFO', - (u'[Transcript Migration] [video-transcripts-migration-process-started-for-course] ' + (u'[Transcript Migration] [run=1] [video-transcripts-migration-process-started-for-course] ' u'[course={}]'.format(course_id)) ), ( 'cms.djangoapps.contentstore.tasks', 'INFO', - (u'[Transcript Migration] [transcripts-migration-process-started-for-video-transcript] ' + (u'[Transcript Migration] [run=1] [transcripts-migration-process-started-for-video-transcript] ' u'[revision=rev-opt-published-only] [video={}] [edx_video_id=test_edx_video_id_2] ' u'[language_code=ge]'.format(self.video_descriptor_2.location)) ), ( 'cms.djangoapps.contentstore.tasks', 'ERROR', - (u'[Transcript Migration] [video-transcript-migration-failed-with-known-exc] ' + (u'[Transcript Migration] [run=1] [video-transcript-migration-failed-with-known-exc] ' u'[revision=rev-opt-published-only] [video={}] [edx_video_id=test_edx_video_id_2] ' u'[language_code=ge]'.format(self.video_descriptor_2.location)) ), ( 'cms.djangoapps.contentstore.tasks', 'INFO', - (u'[Transcript Migration] [transcripts-migration-tasks-submitted] ' + (u'[Transcript Migration] [run=1] [transcripts-migration-tasks-submitted] ' u'[transcripts_count=1] [course={}] ' u'[revision=rev-opt-published-only] [video={}]'.format(course_id, self.video_descriptor_2.location)) ) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 8c25d44957..2778c5eb15 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -84,31 +84,37 @@ VIDEO_LEVEL_TIMEOUT_SECONDS = 300 @chord_task(bind=True) -def task_status_callback(self, results, revision, course_id, video_location): # pylint: disable=unused-argument +def task_status_callback(self, results, revision, # pylint: disable=unused-argument + course_id, command_run, video_location): """ Callback for collating the results of chord. """ transcript_tasks_count = len(list(results())) LOGGER.info( - ("[%s] [video-transcripts-migration-complete-for-a-video] [tasks_count=%s] [course_id=%s] " + ("[%s] [run=%s] [video-transcripts-migration-complete-for-a-video] [tasks_count=%s] [course_id=%s] " "[revision=%s] [video=%s]"), - MIGRATION_LOGS_PREFIX, transcript_tasks_count, course_id, revision, video_location + MIGRATION_LOGS_PREFIX, command_run, transcript_tasks_count, course_id, revision, video_location ) -def enqueue_async_migrate_transcripts_tasks(course_keys, force_update=DEFAULT_FORCE_UPDATE, commit=DEFAULT_COMMIT): +def enqueue_async_migrate_transcripts_tasks(course_keys, + command_run, + force_update=DEFAULT_FORCE_UPDATE, + commit=DEFAULT_COMMIT): """ Fires new Celery tasks for all the input courses or for all courses. Arguments: - course_keys: Command line course ids as list of CourseKey objects, - force_update: Overwrite file in S3. Default is False, - commit: Update S3 or dry-run the command to see which transcripts will be affected. Default is False. + course_keys: Command line course ids as list of CourseKey objects + command_run: A positive number indicating the run counts for transcripts migrations + force_update: Overwrite file in S3. Default is False + commit: Update S3 or dry-run the command to see which transcripts will be affected. Default is False """ kwargs = { 'force_update': force_update, - 'commit': commit + 'commit': commit, + 'command_run': command_run } group([ async_migrate_transcript.s(unicode(course_key), **kwargs) @@ -157,11 +163,12 @@ def async_migrate_transcript(self, course_key, **kwargs): # pylint: disable=un Migrates the transcripts of all videos in a course as a new celery task. """ force_update = kwargs['force_update'] + command_run = kwargs['command_run'] course_videos = get_course_videos(CourseKey.from_string(course_key)) LOGGER.info( - "[%s] [video-transcripts-migration-process-started-for-course] [course=%s]", - MIGRATION_LOGS_PREFIX, course_key + "[%s] [run=%s] [video-transcripts-migration-process-started-for-course] [course=%s]", + MIGRATION_LOGS_PREFIX, command_run, course_key ) for revision, videos in course_videos.items(): @@ -182,27 +189,29 @@ def async_migrate_transcript(self, course_key, **kwargs): # pylint: disable=un callback = task_status_callback.s( revision=revision, course_id=course_key, + command_run=command_run, video_location=video_location ) chord(sub_tasks)(callback) LOGGER.info( - ("[%s] [transcripts-migration-tasks-submitted] " + ("[%s] [run=%s] [transcripts-migration-tasks-submitted] " "[transcripts_count=%s] [course=%s] [revision=%s] [video=%s]"), - MIGRATION_LOGS_PREFIX, len(sub_tasks), course_key, revision, video_location + MIGRATION_LOGS_PREFIX, command_run, len(sub_tasks), course_key, revision, video_location ) else: LOGGER.info( - "[%s] [no-video-transcripts] [course=%s] [revision=%s] [video=%s]", - MIGRATION_LOGS_PREFIX, course_key, revision, video_location + "[%s] [run=%s] [no-video-transcripts] [course=%s] [revision=%s] [video=%s]", + MIGRATION_LOGS_PREFIX, command_run, course_key, revision, video_location ) -def save_transcript_to_storage(edx_video_id, language_code, transcript_content, file_format, force_update): +def save_transcript_to_storage(command_run, edx_video_id, language_code, transcript_content, file_format, force_update): """ Pushes a given transcript's data to django storage. Arguments: + command_run: A positive integer indicating the current run edx_video_id: video ID language_code: language code transcript_content: content of the transcript @@ -227,8 +236,8 @@ def save_transcript_to_storage(edx_video_id, language_code, transcript_content, ) else: LOGGER.info( - "[%s] [do-not-override-existing-transcript] [edx_video_id=%s] [language_code=%s]", - MIGRATION_LOGS_PREFIX, edx_video_id, language_code + "[%s] [run=%s] [do-not-override-existing-transcript] [edx_video_id=%s] [language_code=%s]", + MIGRATION_LOGS_PREFIX, command_run, edx_video_id, language_code ) @@ -245,21 +254,23 @@ def async_migrate_transcript_subtask(self, *args, **kwargs): # pylint: disable= """ success, failure = 'Success', 'Failure' video_location, revision, language_code, force_update = args + command_run = kwargs['command_run'] store = modulestore() video = store.get_item(usage_key=BlockUsageLocator.from_string(video_location), revision=revision) edx_video_id = clean_video_id(video.edx_video_id) if not kwargs['commit']: LOGGER.info( - '[%s] [video-transcript-will-be-migrated] [revision=%s] [video=%s] [edx_video_id=%s] [language_code=%s]', - MIGRATION_LOGS_PREFIX, revision, video_location, edx_video_id, language_code + ('[%s] [run=%s] [video-transcript-will-be-migrated] ' + '[revision=%s] [video=%s] [edx_video_id=%s] [language_code=%s]'), + MIGRATION_LOGS_PREFIX, command_run, revision, video_location, edx_video_id, language_code ) return success LOGGER.info( - ('[%s] [transcripts-migration-process-started-for-video-transcript] [revision=%s] ' + ('[%s] [run=%s] [transcripts-migration-process-started-for-video-transcript] [revision=%s] ' '[video=%s] [edx_video_id=%s] [language_code=%s]'), - MIGRATION_LOGS_PREFIX, revision, video_location, edx_video_id, language_code + MIGRATION_LOGS_PREFIX, command_run, revision, video_location, edx_video_id, language_code ) try: @@ -285,11 +296,12 @@ def async_migrate_transcript_subtask(self, *args, **kwargs): # pylint: disable= store.update_item(video, ModuleStoreEnum.UserID.mgmt_command) LOGGER.info( - '[%s] [generated-edx-video-id] [revision=%s] [video=%s] [edx_video_id=%s] [language_code=%s]', - MIGRATION_LOGS_PREFIX, revision, video_location, edx_video_id, language_code + '[%s] [run=%s] [generated-edx-video-id] [revision=%s] [video=%s] [edx_video_id=%s] [language_code=%s]', + MIGRATION_LOGS_PREFIX, command_run, revision, video_location, edx_video_id, language_code ) save_transcript_to_storage( + command_run=command_run, edx_video_id=edx_video_id, language_code=language_code, transcript_content=transcript_content, @@ -298,23 +310,23 @@ def async_migrate_transcript_subtask(self, *args, **kwargs): # pylint: disable= ) except (NotFoundError, TranscriptsGenerationException, ValCannotCreateError): LOGGER.exception( - ('[%s] [video-transcript-migration-failed-with-known-exc] [revision=%s] [video=%s] ' + ('[%s] [run=%s] [video-transcript-migration-failed-with-known-exc] [revision=%s] [video=%s] ' '[edx_video_id=%s] [language_code=%s]'), - MIGRATION_LOGS_PREFIX, revision, video_location, edx_video_id, language_code + MIGRATION_LOGS_PREFIX, command_run, revision, video_location, edx_video_id, language_code ) return failure except Exception: LOGGER.exception( - ('[%s] [video-transcript-migration-failed-with-unknown-exc] [revision=%s] ' + ('[%s] [run=%s] [video-transcript-migration-failed-with-unknown-exc] [revision=%s] ' '[video=%s] [edx_video_id=%s] [language_code=%s]'), - MIGRATION_LOGS_PREFIX, revision, video_location, edx_video_id, language_code + MIGRATION_LOGS_PREFIX, command_run, revision, video_location, edx_video_id, language_code ) raise LOGGER.info( - ('[%s] [video-transcript-migration-succeeded-for-a-video] [revision=%s] ' + ('[%s] [run=%s] [video-transcript-migration-succeeded-for-a-video] [revision=%s] ' '[video=%s] [edx_video_id=%s] [language_code=%s]'), - MIGRATION_LOGS_PREFIX, revision, video_location, edx_video_id, language_code + MIGRATION_LOGS_PREFIX, command_run, revision, video_location, edx_video_id, language_code ) return success diff --git a/openedx/core/djangoapps/video_config/migrations/0004_transcriptmigrationsetting_command_run.py b/openedx/core/djangoapps/video_config/migrations/0004_transcriptmigrationsetting_command_run.py new file mode 100644 index 0000000000..a7d34199e3 --- /dev/null +++ b/openedx/core/djangoapps/video_config/migrations/0004_transcriptmigrationsetting_command_run.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.13 on 2018-06-25 12:54 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('video_config', '0003_transcriptmigrationsetting'), + ] + + operations = [ + migrations.AddField( + model_name='transcriptmigrationsetting', + name='command_run', + field=models.PositiveIntegerField(default=0), + ), + ] diff --git a/openedx/core/djangoapps/video_config/models.py b/openedx/core/djangoapps/video_config/models.py index bfb2f610ec..5743182584 100644 --- a/openedx/core/djangoapps/video_config/models.py +++ b/openedx/core/djangoapps/video_config/models.py @@ -2,7 +2,7 @@ Configuration models for Video XModule """ from config_models.models import ConfigurationModel -from django.db.models import BooleanField, TextField +from django.db.models import BooleanField, TextField, PositiveIntegerField from opaque_keys.edx.django.models import CourseKeyField @@ -149,6 +149,7 @@ class TranscriptMigrationSetting(ConfigurationModel): default=False, help_text="Flag to force migrate transcripts for the requested courses, overwrite if already present." ) + command_run = PositiveIntegerField(default=0) commit = BooleanField( default=False, help_text="Dry-run or commit." @@ -161,3 +162,11 @@ class TranscriptMigrationSetting(ConfigurationModel): blank=False, help_text="Whitespace-separated list of course keys for which to migrate transcripts." ) + + def increment_run(self): + """ + Increments the run which indicates how many time the mgmt. command has run. + """ + self.command_run += 1 + self.save() + return self.command_run