diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index fb012c8a32..93b94b0509 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -48,7 +48,6 @@ from openedx.core.djangoapps.profile_images.tests.helpers import make_image_file from openedx.core.djangoapps.video_pipeline.config.waffle import ( DEPRECATE_YOUTUBE, ENABLE_DEVSTACK_VIDEO_UPLOADS, - ENABLE_VEM_PIPELINE, waffle_flags ) from openedx.core.djangoapps.video_pipeline.models import VEMPipelineIntegration @@ -504,17 +503,8 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): @patch('boto.s3.connection.S3Connection') def test_send_course_to_vem_pipeline(self, mock_conn, mock_key): """ - Test that if VEM integration pipeline is present and enabled, the upload - goes to VEM pipeline. + Test that uploads always go to VEM S3 bucket by default. """ - vem_pipeline_integration_defaults = { - 'enabled': True, - 'api_url': 'https://video-encode-manager.example.com/api/v1/', - 'service_username': 'vem_pipeline_service_user', - 'client_name': 'vem_pipeline', - } - VEMPipelineIntegration.objects.create(**vem_pipeline_integration_defaults) - files = [{'file_name': 'first.mp4', 'content_type': 'video/mp4'}] mock_key_instances = [ Mock( @@ -537,44 +527,6 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): settings.VIDEO_UPLOAD_PIPELINE['VEM_S3_BUCKET'], validate=False # pylint: disable=unsubscriptable-object ) - @patch('contentstore.views.videos.LOGGER') - @patch('boto.s3.key.Key') - @patch('boto.s3.connection.S3Connection') - def test_vem_pipeline_integration_not_enabled(self, mock_conn, mock_key, mock_logger): - """ - Test that if VEMPipelineIntegration is not enabled and course override waffle flag is not - set to True, the video goes to VEDA bucket. - """ - vem_pipeline_integration_defaults = { - 'enabled': False, - } - VEMPipelineIntegration.objects.create(**vem_pipeline_integration_defaults) - - files = [{'file_name': 'first.mp4', 'content_type': 'video/mp4'}] - mock_key_instances = [ - Mock( - generate_url=Mock( - return_value='http://example.com/url_{}'.format(file_info['file_name']) - ) - ) - for file_info in files - ] - mock_key.side_effect = mock_key_instances - - response = self.client.post( - self.url, - json.dumps({'files': files}), - content_type='application/json' - ) - - self.assertEqual(response.status_code, 200) - mock_conn.return_value.get_bucket.assert_called_once_with( - settings.VIDEO_UPLOAD_PIPELINE['BUCKET'], validate=False # pylint: disable=unsubscriptable-object - ) - mock_logger.info.assert_called_with( - 'Uploading course: {} to VEDA bucket.'.format(self.course.id) - ) - @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') @patch('boto.s3.key.Key') @patch('boto.s3.connection.S3Connection') diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index 743f197148..8977304aea 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -22,7 +22,6 @@ from django.http import FileResponse, HttpResponseNotFound from django.urls import reverse from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_noop -from openedx.core.djangoapps.video_pipeline.models import VEMPipelineIntegration from django.views.decorators.http import require_GET, require_http_methods, require_POST from edxval.api import ( SortDirection, @@ -50,7 +49,6 @@ from openedx.core.djangoapps.video_config.models import VideoTranscriptEnabledFl from openedx.core.djangoapps.video_pipeline.config.waffle import ( DEPRECATE_YOUTUBE, ENABLE_DEVSTACK_VIDEO_UPLOADS, - ENABLE_VEM_PIPELINE, waffle_flags ) from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlagNamespace, WaffleSwitchNamespace @@ -826,20 +824,12 @@ def storage_service_bucket(course_key=None): } conn = s3.connection.S3Connection(**params) - vem_pipeline = VEMPipelineIntegration.current() # We don't need to validate our bucket, it requires a very permissive IAM permission # set since behind the scenes it fires a HEAD request that is equivalent to get_all_keys() # meaning it would need ListObjects on the whole bucket, not just the path used in each # environment (since we share a single bucket for multiple deployments in some configurations) - # - # All the videos should go to VEM by default. VEDA related code will remain in-place - # until its deprecation. - if vem_pipeline and vem_pipeline.enabled: - return conn.get_bucket(settings.VIDEO_UPLOAD_PIPELINE['VEM_S3_BUCKET'], validate=False) - else: - LOGGER.info('Uploading course: {} to VEDA bucket.'.format(course_key)) - return conn.get_bucket(settings.VIDEO_UPLOAD_PIPELINE['BUCKET'], validate=False) + return conn.get_bucket(settings.VIDEO_UPLOAD_PIPELINE['VEM_S3_BUCKET'], validate=False) def storage_service_key(bucket, file_name): diff --git a/openedx/core/djangoapps/video_pipeline/admin.py b/openedx/core/djangoapps/video_pipeline/admin.py index b04f9e45f4..26f354205a 100644 --- a/openedx/core/djangoapps/video_pipeline/admin.py +++ b/openedx/core/djangoapps/video_pipeline/admin.py @@ -13,7 +13,6 @@ from openedx.core.djangoapps.video_pipeline.forms import ( from openedx.core.djangoapps.video_pipeline.models import ( CourseVideoUploadsEnabledByDefault, VEMPipelineIntegration, - VideoPipelineIntegration, VideoUploadsEnabledByDefault ) @@ -33,8 +32,6 @@ class VEMPipelineIntegrationAdmin(ConfigurationModelAdmin): form = VEMPipelineIntegrationAdminForm -admin.site.register(VideoPipelineIntegration, ConfigurationModelAdmin) admin.site.register(VEMPipelineIntegration, VEMPipelineIntegrationAdmin) - admin.site.register(VideoUploadsEnabledByDefault, ConfigurationModelAdmin) admin.site.register(CourseVideoUploadsEnabledByDefault, CourseVideoUploadsEnabledByDefaultAdmin) diff --git a/openedx/core/djangoapps/video_pipeline/api.py b/openedx/core/djangoapps/video_pipeline/api.py index a83888cbb0..8daf8e6e02 100644 --- a/openedx/core/djangoapps/video_pipeline/api.py +++ b/openedx/core/djangoapps/video_pipeline/api.py @@ -9,7 +9,7 @@ from django.core.exceptions import ObjectDoesNotExist from oauth2_provider.models import Application from slumber.exceptions import HttpClientError -from openedx.core.djangoapps.video_pipeline.models import VEMPipelineIntegration, VideoPipelineIntegration +from openedx.core.djangoapps.video_pipeline.models import VEMPipelineIntegration from openedx.core.djangoapps.video_pipeline.utils import create_video_pipeline_api_client log = logging.getLogger(__name__) @@ -72,22 +72,11 @@ def update_3rd_party_transcription_service_credentials(**credentials_payload): error_response, is_updated = {}, False vem_pipeline_integration = VEMPipelineIntegration.current() - veda_pipeline_integration = VideoPipelineIntegration.current() if vem_pipeline_integration.enabled: log.info('Sending transcript credentials to VEM for org: {} and provider: {}'.format( credentials_payload.get('org'), credentials_payload.get('provider') )) error_response, is_updated = send_transcript_credentials(vem_pipeline_integration, credentials_payload) - # If credentials update fail for VEM, return proper error message and discontinue to - # send credentials to VEDA because we are going to try it next time anyway - if not is_updated: - return error_response, is_updated - - if veda_pipeline_integration.enabled: - log.info('Sending transcript credentials to VEDA for org: {} and provider: {}'.format( - credentials_payload.get('org'), credentials_payload.get('provider') - )) - error_response, is_updated = send_transcript_credentials(veda_pipeline_integration, credentials_payload) return error_response, is_updated diff --git a/openedx/core/djangoapps/video_pipeline/management/commands/create_video_pipeline_integration.py b/openedx/core/djangoapps/video_pipeline/management/commands/create_video_pipeline_integration.py deleted file mode 100644 index 269f3f17d1..0000000000 --- a/openedx/core/djangoapps/video_pipeline/management/commands/create_video_pipeline_integration.py +++ /dev/null @@ -1,27 +0,0 @@ -""" -Management command `create_video_pipeline_integration` is used to create video pipeline integration record. -""" - - -from openedx.core.djangoapps.video_pipeline.models import VideoPipelineIntegration -from django.core.management.base import BaseCommand - - -class Command(BaseCommand): - # pylint: disable=missing-docstring - - help = 'Creates the video pipeline integration record.' - - def add_arguments(self, parser): - parser.add_argument('client_name') - parser.add_argument('api_url') - parser.add_argument('service_username') - parser.add_argument('--enabled', dest='enabled', action='store_true') - - def handle(self, **fields): - VideoPipelineIntegration.objects.get_or_create( - client_name=fields['client_name'], - api_url=fields['api_url'], - service_username=fields['service_username'], - enabled=fields['enabled'], - ) diff --git a/openedx/core/djangoapps/video_pipeline/management/commands/tests/test_create_video_pipeline_integration.py b/openedx/core/djangoapps/video_pipeline/management/commands/tests/test_create_video_pipeline_integration.py deleted file mode 100644 index c7876b94d5..0000000000 --- a/openedx/core/djangoapps/video_pipeline/management/commands/tests/test_create_video_pipeline_integration.py +++ /dev/null @@ -1,76 +0,0 @@ -""" -Tests for create_video_pipeline_integration management command. -""" - - -import ddt -from django.core.management import call_command -from django.test import TestCase -from openedx.core.djangoapps.video_pipeline.models import VideoPipelineIntegration - - -@ddt.ddt -class CreateVideoPipelineIntegration(TestCase): - """ - Management command test class. - """ - def setUp(self): - super(CreateVideoPipelineIntegration, self).setUp() - - def assert_integration_created(self, args, options): - """ - Verify that the integration record was created. - """ - integration = VideoPipelineIntegration.current() - - for index, attr in enumerate(('client_name', 'api_url', 'service_username')): - self.assertEqual(args[index], getattr(integration, attr)) - - self.assertEqual(integration.enabled, options.get('enabled')) - - @ddt.data( - ( - [ - 'veda', - 'http://veda.edx.org/api/', - 'veda_service_user', - ], - {'enabled': False} - ), - ( - [ - 'veda', - 'http://veda.edx.org/api/', - 'veda_service_user', - ], - {'enabled': True} - ), - ) - @ddt.unpack - def test_integration_creation(self, args, options): - """ - Verify that the create_video_pipeline_integration command works as expected. - """ - call_command('create_video_pipeline_integration', *args, **options) - self.assert_integration_created(args, options) - - def test_idempotency(self): - """ - Verify that the command can be run repeatedly with the same args and options without any ill effects. - """ - args = [ - 'veda', - 'http://veda.edx.org/api/', - 'veda_service_user', - ] - options = {'enabled': False} - - call_command('create_video_pipeline_integration', *args, **options) - self.assert_integration_created(args, options) - - # Verify that the command is idempotent - call_command('create_video_pipeline_integration', *args, **options) - self.assert_integration_created(args, options) - - # Verify that only one record exists - self.assertEqual(VideoPipelineIntegration.objects.count(), 1) diff --git a/openedx/core/djangoapps/video_pipeline/tests/mixins.py b/openedx/core/djangoapps/video_pipeline/tests/mixins.py index 37c6f2f008..f3902830d0 100644 --- a/openedx/core/djangoapps/video_pipeline/tests/mixins.py +++ b/openedx/core/djangoapps/video_pipeline/tests/mixins.py @@ -4,19 +4,13 @@ Mixins to test video pipeline integration. from oauth2_provider.models import Application -from openedx.core.djangoapps.video_pipeline.models import VEMPipelineIntegration, VideoPipelineIntegration +from openedx.core.djangoapps.video_pipeline.models import VEMPipelineIntegration class VideoPipelineMixin(object): """ - Utility for working with the video pipelines (VEDA and VEM) service during testing. + Utility for working with the VEM video pipeline service during testing. """ - veda_pipeline_integration_defaults = { - 'enabled': True, - 'api_url': 'https://video-pipeline.example.com/api/v1/', - 'service_username': 'cms_veda_pipeline_service_user', - 'client_name': 'veda_pipeline' - } vem_pipeline_integration_defaults = { 'enabled': True, @@ -30,14 +24,6 @@ class VideoPipelineMixin(object): 'https://video-encode-manager.example.com/api/v1/logout ' \ 'https://video-encode-manager.example.com/api/v1/redirect' - def create_video_pipeline_integration(self, **kwargs): - """ - Creates a new `VideoPipelineIntegration` record with `veda_pipeline_integration_defaults`, - and it can be updated with any provided overrides. - """ - fields = dict(self.veda_pipeline_integration_defaults, **kwargs) - return VideoPipelineIntegration.objects.create(**fields) - def create_vem_pipeline_integration(self, **kwargs): """ Creates a new `VEMPipelineIntegration` record with `vem_pipeline_integration_defaults`, @@ -46,15 +32,15 @@ class VideoPipelineMixin(object): fields = dict(self.vem_pipeline_integration_defaults, **kwargs) return VEMPipelineIntegration.objects.create(**fields) - def create_video_pipeline_oauth_client(self, user, vem_enabled=False, **kwargs): + def create_video_pipeline_oauth_client(self, user, **kwargs): """ Creates a new `Client` record with `video_pipeline_oauth_client_defaults`, and it can be updated with any provided overrides. """ video_pipeline_oauth_client_defaults = { - 'name': 'vem_pipeline' if vem_enabled else 'veda_pipeline', + 'name': 'vem_pipeline', 'redirect_uris': self.request_uris, - 'client_id': 'vem_client_id' if vem_enabled else 'video_pipeline_client_id', + 'client_id': 'vem_client_id', 'client_secret': 'video_pipeline_client_secret', 'client_type': Application.CLIENT_CONFIDENTIAL } diff --git a/openedx/core/djangoapps/video_pipeline/tests/test_api.py b/openedx/core/djangoapps/video_pipeline/tests/test_api.py index d6ab2e8e17..38b5f34074 100644 --- a/openedx/core/djangoapps/video_pipeline/tests/test_api.py +++ b/openedx/core/djangoapps/video_pipeline/tests/test_api.py @@ -21,55 +21,22 @@ class TestAPIUtils(VideoPipelineMixin, TestCase): """ def setUp(self): """ - Setup VEDA and VEM oauth clients. + Setup VEM oauth client. """ - self.add_veda_client() self.add_vem_client() - def add_veda_client(self): - """ - Creates a VEDA oauth client - """ - self.veda_pipeline_integration = self.create_video_pipeline_integration() - self.veda_user = UserFactory(username=self.veda_pipeline_integration.service_username) - self.veda_oauth_client = self.create_video_pipeline_oauth_client(user=self.veda_user) - def add_vem_client(self): """ Creates a VEM oauth client """ self.vem_pipeline_integration = self.create_vem_pipeline_integration() self.vem_user = UserFactory(username=self.vem_pipeline_integration.service_username) - self.vem_oauth_client = self.create_video_pipeline_oauth_client(user=self.vem_user, vem_enabled=True) + self.vem_oauth_client = self.create_video_pipeline_oauth_client(user=self.vem_user) - @patch('openedx.core.djangoapps.video_pipeline.api.log') - @patch('openedx.core.djangoapps.video_pipeline.utils.OAuthAPIClient') - def test_update_transcription_service_credentials_with_one_integration_disabled(self, mock_client, mock_logger): + def test_update_transcription_service_credentials_with_vem_disabled(self): """ - Test that updating the credentials when one of the service integration is disabled, allows - the credentials to be updated for other pipeline. + Test updating the credentials when VEM integration is disabled. """ - mock_client.request.return_value.ok = True - credentials_payload = { - 'org': 'mit', 'provider': 'ABC Provider', 'api_key': '61c56a8d0' - } - - self.veda_pipeline_integration.enabled = False - self.veda_pipeline_integration.save() - __, is_updated = update_3rd_party_transcription_service_credentials(**credentials_payload) - mock_logger.info.assert_called_with('Sending transcript credentials to VEM for org: {} and provider: {}'.format( - credentials_payload.get('org'), credentials_payload.get('provider') - )) - self.assertTrue(is_updated) - - def test_update_transcription_service_credentials_with_both_integration_disabled(self): - """ - Test updating the credentials when both service integration are disabled. - """ - # Disabling VEDA - self.veda_pipeline_integration.enabled = False - self.veda_pipeline_integration.save() - # Disabling VEM self.vem_pipeline_integration.enabled = False self.vem_pipeline_integration.save() @@ -77,16 +44,6 @@ class TestAPIUtils(VideoPipelineMixin, TestCase): __, is_updated = update_3rd_party_transcription_service_credentials() self.assertFalse(is_updated) - def test_update_transcription_service_credentials_when_one_service_fails(self): - """ - Test that if one of the transcription service fails to update credentials, - response from `update_3rd_party_transcription_service_credentials` is False. - """ - self.vem_pipeline_integration.client_name = 'non_existent_client' - self.vem_pipeline_integration.save() - __, is_updated = update_3rd_party_transcription_service_credentials() - self.assertFalse(is_updated) - @ddt.data( { 'username': 'Jason_cielo_24', @@ -116,9 +73,6 @@ class TestAPIUtils(VideoPipelineMixin, TestCase): mock_logger.info.assert_any_call('Sending transcript credentials to VEM for org: {} and provider: {}'.format( credentials_payload.get('org'), credentials_payload.get('provider') )) - mock_logger.info.assert_any_call('Sending transcript credentials to VEDA for org: {} and provider: {}'.format( - credentials_payload.get('org'), credentials_payload.get('provider') - )) @patch('openedx.core.djangoapps.video_pipeline.api.log') @patch('openedx.core.djangoapps.video_pipeline.utils.OAuthAPIClient')