diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index a42f47b6960043ab82d92e3349a2f152a06d138c..51a7e4d7320e24ad509f968518582ac17b69cf40 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -291,18 +291,3 @@ def is_item_in_course_tree(item): ancestor = ancestor.get_parent() return ancestor is not None - - -def get_course_hash_value(course_key): - """ - Returns a hash value for the given course key. - - If course key is None, function returns an out of bound value which will - never satisfy the vem_enabled_courses_percentage condition - """ - out_of_bound_value = 100 - if course_key: - m = hashlib.md5(str(course_key).encode()) - return int(m.hexdigest(), base=16) % 100 - - return out_of_bound_value diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index 558c18a5d00e2b26deeb1ecdf41794fe39c6b6da..ba8a753289a862ec1802eb4ebc7a15c06689d726 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -502,49 +502,16 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): @patch('boto.s3.key.Key') @patch('boto.s3.connection.S3Connection') - @override_flag(waffle_flags()[ENABLE_VEM_PIPELINE].namespaced_flag_name, active=True) - def test_enable_vem_pipeline(self, mock_conn, mock_key): + def test_send_course_to_vem_pipeline(self, mock_conn, mock_key): """ - Test that if VEM pipeline is enabled, objects are uploaded to the correct s3 bucket - even if course_hash_value is bigger than vem_enabled_courses_percentage. - """ - 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['VEM_S3_BUCKET'], validate=False # pylint: disable=unsubscriptable-object - ) - - @patch('contentstore.views.videos.get_course_hash_value', Mock(return_value=50)) - @patch('contentstore.views.videos.LOGGER') - @patch('boto.s3.key.Key') - @patch('boto.s3.connection.S3Connection') - def test_send_course_to_vem_pipeline(self, mock_conn, mock_key, mock_logger): - """ - Test that if course hash value lies under the VEM config `vem_enabled_courses_percentage` - value, then video for that course is uploaded to VEM. + Test that if VEM integration pipeline is present and enabled, the upload + goes to VEM pipeline. """ 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', - 'vem_enabled_courses_percentage': 100 } VEMPipelineIntegration.objects.create(**vem_pipeline_integration_defaults) @@ -569,18 +536,17 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): mock_conn.return_value.get_bucket.assert_called_once_with( settings.VIDEO_UPLOAD_PIPELINE['VEM_S3_BUCKET'], validate=False # pylint: disable=unsubscriptable-object ) - mock_logger.info.assert_called_with('Uploading course: {} to VEM bucket.'.format(self.course.id)) - @patch('contentstore.views.videos.get_course_hash_value', Mock(return_value=50)) + @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): + 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, 'vem_enabled_courses_percentage': 100 + 'enabled': False, } VEMPipelineIntegration.objects.create(**vem_pipeline_integration_defaults) @@ -605,6 +571,9 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): 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') diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index 5717323da58719fc9a29abd5c1f634ee6228f306..2c60838d239d179b5b23c0de0548f2b77fc10022 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -44,7 +44,6 @@ from pytz import UTC from contentstore.models import VideoUploadConfig from contentstore.utils import reverse_course_url from contentstore.video_utils import validate_video_image -from contentstore.views.helpers import get_course_hash_value from edxmako.shortcuts import render_to_response from openedx.core.djangoapps.video_config.models import VideoTranscriptEnabledFlag from openedx.core.djangoapps.video_pipeline.config.waffle import ( @@ -826,19 +825,18 @@ def storage_service_bucket(course_key=None): conn = s3.connection.S3Connection(**params) vem_pipeline = VEMPipelineIntegration.current() - course_hash_value = get_course_hash_value(course_key) - - vem_override = course_key and waffle_flags()[ENABLE_VEM_PIPELINE].is_enabled(course_key) - allow_course_to_use_vem = vem_pipeline.enabled and course_hash_value < vem_pipeline.vem_enabled_courses_percentage # 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) - if vem_override or allow_course_to_use_vem: - LOGGER.info('Uploading course: {} to VEM bucket.'.format(course_key)) + # + # 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) diff --git a/openedx/core/djangoapps/video_pipeline/admin.py b/openedx/core/djangoapps/video_pipeline/admin.py index b04f9e45f4143b9bfee97057e318d1b32224c46c..cec80cbe9ca4f6385e8421708a3f61b8af144f46 100644 --- a/openedx/core/djangoapps/video_pipeline/admin.py +++ b/openedx/core/djangoapps/video_pipeline/admin.py @@ -34,7 +34,7 @@ class VEMPipelineIntegrationAdmin(ConfigurationModelAdmin): admin.site.register(VideoPipelineIntegration, ConfigurationModelAdmin) -admin.site.register(VEMPipelineIntegration, VEMPipelineIntegrationAdmin) +admin.site.register(VEMPipelineIntegration, ConfigurationModelAdmin) admin.site.register(VideoUploadsEnabledByDefault, ConfigurationModelAdmin) admin.site.register(CourseVideoUploadsEnabledByDefault, CourseVideoUploadsEnabledByDefaultAdmin) diff --git a/openedx/core/djangoapps/video_pipeline/forms.py b/openedx/core/djangoapps/video_pipeline/forms.py index 5704bfe62de352cfe4f12d17edf703d03451567e..b2a2ca91996b549a5e5595d4fac78bccbc88e81c 100644 --- a/openedx/core/djangoapps/video_pipeline/forms.py +++ b/openedx/core/djangoapps/video_pipeline/forms.py @@ -4,7 +4,10 @@ Defines a form to provide validations for course-specific configuration. from django import forms from openedx.core.djangoapps.video_config.forms import CourseSpecificFlagAdminBaseForm -from openedx.core.djangoapps.video_pipeline.models import CourseVideoUploadsEnabledByDefault +from openedx.core.djangoapps.video_pipeline.models import ( + CourseVideoUploadsEnabledByDefault, + VEMPipelineIntegration, +) class CourseVideoUploadsEnabledByDefaultAdminForm(CourseSpecificFlagAdminBaseForm): @@ -19,14 +22,8 @@ class CourseVideoUploadsEnabledByDefaultAdminForm(CourseSpecificFlagAdminBaseFor class VEMPipelineIntegrationAdminForm(forms.ModelForm): """ - Form for VEM Pipeline Integration Admin class + Form for VEM Pipeline Integration Admin class. """ - - def clean_vem_enabled_courses_percentage(self): - """ - Validates that vem_enabled_courses_percentage lies between 0 to 100. - """ - vem_enabled_courses_percentage = self.cleaned_data['vem_enabled_courses_percentage'] - if vem_enabled_courses_percentage < 0 or vem_enabled_courses_percentage > 100: - raise forms.ValidationError('Invalid percentage, the value must be between 0 and 100') - return vem_enabled_courses_percentage + class Meta(object): + model = VEMPipelineIntegration + fields = '__all__' diff --git a/openedx/core/djangoapps/video_pipeline/models.py b/openedx/core/djangoapps/video_pipeline/models.py index 84a6f2074aeca111e3bce6f7b44373417e946120..4911a15ae0fec078ed69c5bef539eaa60a78530a 100644 --- a/openedx/core/djangoapps/video_pipeline/models.py +++ b/openedx/core/djangoapps/video_pipeline/models.py @@ -72,11 +72,6 @@ class VEMPipelineIntegration(ConfigurationModel): help_text=_('Username created for VEM Integration, e.g. vem_service_user.') ) - vem_enabled_courses_percentage = models.IntegerField( - default=0, - help_text=_('Percentage of courses allowed to use VEM pipeline') - ) - def get_service_user(self): User = get_user_model() # pylint: disable=invalid-name return User.objects.get(username=self.service_username)