Address feedback, fix tests, rename transcript preferences.

VEDA_OS01/admin.py

@@ -2,7 +2,7 @@ from django.contrib import admin
 
 from VEDA_OS01.models import (
     Course, Video, Encode, URL, Destination, Institution, VedaUpload,
-    TranscriptPreferences, TranscriptProcessMetadata
+    TranscriptCredentials, TranscriptProcessMetadata
 )
 
 
@@ -109,8 +109,8 @@ class VideoUploadAdmin(admin.ModelAdmin):
     ]
 
 
-class TranscriptPreferencesAdmin(admin.ModelAdmin):
-    model = TranscriptPreferences
+class TranscriptCredentialsAdmin(admin.ModelAdmin):
+    model = TranscriptCredentials
 
 
 class TranscriptProcessMetadataAdmin(admin.ModelAdmin):
@@ -124,5 +124,5 @@ admin.site.register(URL, URLAdmin)
 admin.site.register(Destination, DestinationAdmin)
 admin.site.register(Institution, InstitutionAdmin)
 admin.site.register(VedaUpload, VideoUploadAdmin)
-admin.site.register(TranscriptPreferences, TranscriptPreferencesAdmin)
+admin.site.register(TranscriptCredentials, TranscriptCredentialsAdmin)
 admin.site.register(TranscriptProcessMetadata, TranscriptProcessMetadataAdmin)

VEDA_OS01/models.py

@@ -619,9 +619,9 @@ class VedaUpload (models.Model):
         )
 
 
-class TranscriptPreferences(TimeStampedModel):
+class TranscriptCredentials(TimeStampedModel):
     """
-    Model to contain third party transcription service provider preferances.
+    Model to contain third party transcription service provider preferences.
     """
     org = models.CharField(
         'Organization',
@@ -634,7 +634,7 @@ class TranscriptPreferences(TimeStampedModel):
 
     class Meta:
         unique_together = ('org', 'provider')
-        verbose_name_plural = 'Transcript preferences'
+        verbose_name_plural = 'Transcript Credentials'
 
     def __unicode__(self):
         return u'{org} - {provider}'.format(org=self.org, provider=self.provider)
@@ -649,7 +649,7 @@ class TranscriptProcessMetadata(TimeStampedModel):
     process_id = models.CharField('Process id', max_length=255)
     # To keep track of 3Play Translations.
     translation_id = models.CharField('Translation id', max_length=255, null=True, blank=True)
-    lang_code = models.CharField('Language code', max_length=3)
+    lang_code = models.CharField('Language code', max_length=8)
     status = models.CharField(
         'Transcript status',
         max_length=50,

VEDA_OS01/tests/test_transcripts.py

@@ -17,7 +17,7 @@ from rest_framework import status
 from rest_framework.test import APITestCase
 
 from VEDA_OS01 import transcripts, utils
-from VEDA_OS01.models import (Course, TranscriptPreferences,
+from VEDA_OS01.models import (Course, TranscriptCredentials,
                               TranscriptProcessMetadata, TranscriptProvider,
                               TranscriptStatus, Video)
 
@@ -108,7 +108,7 @@ class Cielo24TranscriptTests(APITestCase):
             **VIDEO_DATA
         )
 
-        self.transcript_prefs = TranscriptPreferences.objects.create(
+        self.transcript_prefs = TranscriptCredentials.objects.create(
             **TRANSCRIPT_PREFERENCES
         )
 
@@ -142,10 +142,11 @@ class Cielo24TranscriptTests(APITestCase):
         REQUEST_PARAMS['video_id'] = self.video.studio_id
 
     @data(
-        {'url': 'cielo24/transcript_completed', 'status_code': 404},
-        {'url': None, 'status_code': 200},
+        ('cielo24/transcript_completed', 404),
+        (None, 200),
     )
     @unpack
+    @patch('VEDA_OS01.transcripts.CIELO24_TRANSCRIPT_COMPLETED.send_robust', Mock(return_value=None))
     def test_provider(self, url, status_code):
         """
         Verify that only valid provider requests are allowed .
@@ -157,19 +158,26 @@ class Cielo24TranscriptTests(APITestCase):
         self.assertEqual(response.status_code, status_code)
 
     @data(
-        {'params': {}},
-        {'params': {'job_id': 1}},
-        {'params': {'job_id': 2, 'lang_code': 'en'}},
-        {'params': {'job_id': 3, 'lang_code': 'ar', 'org': 'edx'}}
+        ({}, ['job_id', 'lang_code', 'org', 'video_id']),
+        ({'job_id': 1}, ['lang_code', 'org', 'video_id']),
+        ({'job_id': 2, 'lang_code': 'en'}, ['org', 'video_id']),
+        ({'job_id': 3, 'lang_code': 'ar', 'org': 'edx'}, ['video_id']),
     )
     @unpack
-    def test_missing_required_params(self, params):
+    @patch('VEDA_OS01.transcripts.LOGGER')
+    def test_missing_required_params(self, params, logger_params, mock_logger):
         """
         Verify that 400 response is recevied if any required param is missing.
         """
         response = self.client.get(self.url, params)
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        mock_logger.warning.assert_called_with(
+            '[CIELO24 HANDLER] Required params are missing %s',
+            logger_params,
+        )
 
+    @responses.activate
+    @patch('VEDA_OS01.transcripts.CIELO24_TRANSCRIPT_COMPLETED.send_robust', Mock(return_value=None))
     def test_transcript_callback_get_request(self):
         """
         Verify that transcript callback get request is working as expected.
@@ -263,7 +271,7 @@ class Cielo24TranscriptTests(APITestCase):
             with self.assertRaises(transcripts.TranscriptConversionError) as conversion_exception:
                 transcripts.cielo24_transcript_callback(None, **REQUEST_PARAMS)
                 mock_logger.exception.assert_called_with(
-                    '[CIELO24 TRANSCRIPTS] Request failed for video=%s -- lang=%s -- job_id=%s -- message=%s',
+                    '[CIELO24 TRANSCRIPTS] Request failed for video=%s -- lang=%s -- job_id=%s.',
                     REQUEST_PARAMS['video_id'],
                     REQUEST_PARAMS['lang_code'],
                     REQUEST_PARAMS['job_id']
@@ -331,7 +339,7 @@ class ThreePlayTranscriptionCallbackTest(APITestCase):
             **VIDEO_DATA
         )
 
-        self.transcript_prefs = TranscriptPreferences.objects.create(
+        self.transcript_prefs = TranscriptCredentials.objects.create(
             org=self.org,
             provider=TranscriptProvider.THREE_PLAY,
             api_key='insecure_api_key',
@@ -710,7 +718,7 @@ class ThreePlayTranscriptionCallbackTest(APITestCase):
             # request - 5
             {
                 'url': transcripts.THREE_PLAY_ORDER_TRANSLATION_URL.format(file_id=self.file_id),
-                'body': urllib.urlencode({
+                'body': json.dumps({
                     'apikey': self.transcript_prefs.api_key,
                     'api_secret_key': self.transcript_prefs.api_secret,
                     'translation_service_id': 30,
@@ -1240,14 +1248,12 @@ class ThreePlayTranscriptionCallbackTest(APITestCase):
 
             ],
             {
-                'method': 'error',
+                'method': 'exception',
                 'args': (
-                    '[3PlayMedia Task] Translation download failed for video=%s -- lang_code=%s -- process_id=%s -- '
-                    'status=%s',
+                    '[3PlayMedia Task] Translation download failed for video=%s -- lang_code=%s -- process_id=%s.',
                     VIDEO_DATA['studio_id'],
                     'ro',
-                    '112233',
-                    400,
+                    '112233'
                 )
             },
             TranscriptStatus.IN_PROGRESS

VEDA_OS01/transcripts.py

@@ -20,7 +20,7 @@ from rest_framework.views import APIView
 
 from control.veda_val import VALAPICall
 from VEDA_OS01 import utils
-from VEDA_OS01.models import (TranscriptPreferences, TranscriptProcessMetadata,
+from VEDA_OS01.models import (TranscriptCredentials, TranscriptProcessMetadata,
                               TranscriptProvider, TranscriptStatus,
                               VideoStatus)
 
@@ -29,11 +29,6 @@ requests.packages.urllib3.disable_warnings(InsecurePlatformWarning)
 logging.basicConfig()
 LOGGER = logging.getLogger(__name__)
 
-VALID_TRANSCRIPT_PROVIDERS = [
-    '',
-    '',
-]
-
 # 3PlayMedia possible send-along statuses for a transcription callback.
 COMPLETE = 'complete'
 ERROR = 'error'
@@ -116,9 +111,13 @@ class Cielo24CallbackHandlerView(APIView):
         """
         Handle Cielo24 callback request.
         """
-        attrs = ('job_id', 'lang_code', 'org', 'video_id')
-        if not all([attr in request.query_params for attr in attrs]):
-            LOGGER.warn('[CIELO24 HANDLER] Required params are missing %s', request.query_params.keys())
+        required_attrs = ('job_id', 'lang_code', 'org', 'video_id')
+        missing = [attr for attr in required_attrs if attr not in request.query_params.keys()]
+        if missing:
+            LOGGER.warning(
+                '[CIELO24 HANDLER] Required params are missing %s',
+                missing,
+            )
             return Response({}, status=status.HTTP_400_BAD_REQUEST)
 
         CIELO24_TRANSCRIPT_COMPLETED.send_robust(
@@ -155,14 +154,14 @@ def cielo24_transcript_callback(sender, **kwargs):
         job_id
     )
 
-    # get transcript preferences for an organization
+    # get transcript credentials for an organization
     try:
-        transcript_prefs = TranscriptPreferences.objects.get(
+        transcript_prefs = TranscriptCredentials.objects.get(
             org=org,
             provider=TranscriptProvider.CIELO24,
         )
-    except TranscriptPreferences.DoesNotExist:
-        LOGGER.exception('[CIELO24 TRANSCRIPTS] Unable to get transcript preferences for job_id=%s', job_id)
+    except TranscriptCredentials.DoesNotExist:
+        LOGGER.exception('[CIELO24 TRANSCRIPTS] Unable to get transcript credentials for job_id=%s', job_id)
 
     # mark the transcript for a particular language as ready
     try:
@@ -177,7 +176,7 @@ def cielo24_transcript_callback(sender, **kwargs):
             job_id
         )
 
-    # if transcript preferences are missing then we can do nothing
+    # if transcript credentials are missing then we can do nothing
     if not transcript_prefs and process_metadata:
         process_metadata.status = TranscriptStatus.FAILED
         process_metadata.save()
@@ -211,7 +210,7 @@ def cielo24_transcript_callback(sender, **kwargs):
             sjson_file_name = upload_sjson_to_s3(CONFIG, sjson)
         except Exception:
             LOGGER.exception(
-                '[CIELO24 TRANSCRIPTS] Request failed for video=%s -- lang=%s -- job_id=%s',
+                '[CIELO24 TRANSCRIPTS] Request failed for video=%s -- lang=%s -- job_id=%s.',
                 video_id,
                 lang_code,
                 job_id
@@ -426,7 +425,7 @@ def order_translations(file_id, api_key, api_secret, target_languages):
             continue
 
         # 2 - At this point, we've got our service ready to use. Now, place an order for the translation.
-        response = requests.post(THREE_PLAY_ORDER_TRANSLATION_URL.format(file_id=file_id), data={
+        response = requests.post(THREE_PLAY_ORDER_TRANSLATION_URL.format(file_id=file_id), json={
             'apikey': api_key,
             'api_secret_key': api_secret,
             'translation_service_id': translation_service_id,
@@ -465,6 +464,7 @@ def order_translations(file_id, api_key, api_secret, target_languages):
 @django.dispatch.receiver(THREE_PLAY_TRANSCRIPTION_DONE, dispatch_uid="three_play_transcription_done")
 def three_play_transcription_callback(sender, **kwargs):
     """
+    This is a receiver for 3Play Media callback signal.
 
     Arguments:
         sender: sender of the signal
@@ -502,8 +502,8 @@ def three_play_transcription_callback(sender, **kwargs):
     if state == COMPLETE:
         # Indicates that the default video speech transcription has been done successfully.
         try:
-            transcript_secrets = TranscriptPreferences.objects.get(org=org, provider=TranscriptProvider.THREE_PLAY)
-        except TranscriptPreferences.DoesNotExist:
+            transcript_secrets = TranscriptCredentials.objects.get(org=org, provider=TranscriptProvider.THREE_PLAY)
+        except TranscriptCredentials.DoesNotExist:
             # Fail the process
             process.status = TranscriptStatus.FAILED
             process.save()
@@ -676,8 +676,8 @@ def retrieve_three_play_translations():
         org = utils.extract_course_org(course_id=course_id)
 
         try:
-            three_play_secrets = TranscriptPreferences.objects.get(org=org, provider=TranscriptProvider.THREE_PLAY)
-        except TranscriptPreferences.DoesNotExist:
+            three_play_secrets = TranscriptCredentials.objects.get(org=org, provider=TranscriptProvider.THREE_PLAY)
+        except TranscriptCredentials.DoesNotExist:
             LOGGER.exception(
                 u'[3PlayMedia Task] 3Play secrets not found for video=%s -- lang_code=%s -- process_id=%s',
                 translation_process.video.studio_id,
@@ -720,22 +720,19 @@ def retrieve_three_play_translations():
             continue
 
         if translation_status['state'] == 'complete':
-            translation_download_url = utils.build_url(
-                THREE_PLAY_TRANSLATION_DOWNLOAD_URL.format(
-                    file_id=translation_process.process_id,
-                    translation_id=translation_process.translation_id,
-                ),
-                apikey=three_play_secrets.api_key
-            )
-            response = requests.get(translation_download_url)
-            if not response.ok:
-                LOGGER.error(
-                    u'[3PlayMedia Task] Translation download failed for video=%s -- lang_code=%s -- process_id=%s -- '
-                    u'status=%s',
+            try:
+                response = fetch_srt_data(
+                    url=THREE_PLAY_TRANSLATION_DOWNLOAD_URL.format(
+                        file_id=translation_process.process_id, translation_id=translation_process.translation_id
+                    ),
+                    apikey=three_play_secrets.api_key,
+                )
+            except TranscriptFetchError:
+                LOGGER.exception(
+                    u'[3PlayMedia Task] Translation download failed for video=%s -- lang_code=%s -- process_id=%s.',
                     translation_process.video.studio_id,
                     translation_process.lang_code,
-                    translation_process.process_id,
-                    response.status_code,
+                    translation_process.process_id
                 )
                 continue
 
@@ -743,7 +740,7 @@ def retrieve_three_play_translations():
             # ValueError if its a valid response, otherwise it'll be json
             # response in result of an error.
             try:
-                json.loads(response.text)
+                json.loads(response)
                 translation_process.status = TranscriptStatus.FAILED
                 translation_process.save()
                 LOGGER.error(
@@ -762,7 +759,7 @@ def retrieve_three_play_translations():
             translation_process.save()
 
             try:
-                sjson_transcript = convert_srt_to_sjson(response.text)
+                sjson_transcript = convert_srt_to_sjson(response)
                 sjson_file = upload_sjson_to_s3(CONFIG, sjson_transcript)
             except Exception:
                 # in case of any exception, log and raise.

control/veda_deliver.py

@@ -18,7 +18,7 @@ from control_env import *
 from veda_deliver_cielo import Cielo24Transcript
 from veda_deliver_youtube import DeliverYoutube
 from VEDA_OS01 import utils
-from VEDA_OS01.models import (TranscriptPreferences, TranscriptProvider,
+from VEDA_OS01.models import (TranscriptCredentials, TranscriptProvider,
                               VideoStatus)
 from VEDA_OS01.utils import build_url
 from veda_utils import ErrorObject, Metadata, Output, VideoProto
@@ -534,8 +534,8 @@ class VedaDelivery:
         org = utils.extract_course_org(self.video_proto.platform_course_url[0])
 
         try:
-            api_key = TranscriptPreferences.objects.get(org=org, provider=self.video_query.provider).api_key
-        except TranscriptPreferences.DoesNotExist:
+            api_key = TranscriptCredentials.objects.get(org=org, provider=self.video_query.provider).api_key
+        except TranscriptCredentials.DoesNotExist:
             LOGGER.warn('[cielo24] Unable to find api_key for org=%s', org)
             return None
 
@@ -578,7 +578,7 @@ class VedaDelivery:
             # Picks the first course from the list as there may be multiple
             # course runs in that list (i.e. all having the same org).
             org = utils.extract_course_org(self.video_proto.platform_course_url[0])
-            transcript_secrets = TranscriptPreferences.objects.get(org=org, provider=self.video_query.provider)
+            transcript_secrets = TranscriptCredentials.objects.get(org=org, provider=self.video_query.provider)
 
             # update transcript status for video in edx-val
             VALAPICall(video_proto=None, val_status=None).update_video_status(
@@ -612,7 +612,7 @@ class VedaDelivery:
             )
             three_play_media.generate_transcripts()
 
-        except TranscriptPreferences.DoesNotExist:
+        except TranscriptCredentials.DoesNotExist:
             LOGGER.warning(
                 'Transcript preference is not found for provider=%s, video=%s',
                 self.video_query.provider,

control/veda_deliver_3play.py

@@ -102,11 +102,7 @@ class ThreePLayMediaClient(object):
             callback_url=self.callback_url,
         )
         upload_url = build_url(self.base_url, self.upload_media_file_url)
-        response = requests.post(
-            url=upload_url,
-            data=json.dumps(payload),
-            headers={'Content-Type': 'application/json'}
-        )
+        response = requests.post(url=upload_url, json=payload)
 
         if not response.ok:
             raise ThreePlayMediaPerformTranscriptionError(

control/veda_file_discovery.py

@@ -7,7 +7,7 @@ import boto.s3
 from boto.exception import S3ResponseError, S3DataError
 import yaml
 
-from VEDA_OS01.models import TranscriptPreferences
+from VEDA_OS01.models import TranscriptCredentials
 from VEDA_OS01.utils import extract_course_org
 
 try:
@@ -237,12 +237,12 @@ class FileDiscovery(object):
         # Make decision if this video needs the transcription as well.
         try:
             transcript_preferences = json.loads(transcript_preferences)
-            TranscriptPreferences.objects.get(
+            TranscriptCredentials.objects.get(
                 org=extract_course_org(course_url),
                 provider=transcript_preferences.get('provider')
             )
             process_transcription = True
-        except (TypeError, TranscriptPreferences.DoesNotExist):
+        except (TypeError, TranscriptCredentials.DoesNotExist):
             # when the preferences are not set OR these are set to some data in invalid format OR these don't
             # have associated 3rd party transcription provider API keys.
             process_transcription = False

instance_config.yaml

@@ -3,23 +3,23 @@
 # Database information
 # ---
 
-# SANDBOX
-#DATABASES:
-#    default:
-#        ENGINE: django.db.backends.sqlite3
-#        NAME: sandbox.db
-
-## PRODUCTION
+#SANDBOX
 DATABASES:
     default:
-        ENGINE: 'django.db.backends.mysql'
-        NAME:
-        USER:
-        PASSWORD:
-        HOST:
-        PORT: '3306'
-
-django_secret_key: ""
+        ENGINE: django.db.backends.sqlite3
+        NAME: sandbox.db
+
+### PRODUCTION
+#DATABASES:
+#    default:
+#        ENGINE: 'django.db.backends.mysql'
+#        NAME:
+#        USER:
+#        PASSWORD:
+#        HOST:
+#        PORT: '3306'
+
+django_secret_key: insecure_secret_key
 
 # Django DEBUG global
 # (set to false in prod)