From 2ae2fbd4f5741ce0ab3fc28976387b7f599ef2d1 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Mon, 13 Mar 2023 13:32:19 +0500 Subject: [PATCH] fix: removing s3boto storage backend. (#31876) * fix: removing s3boto storage backend --- .../contentstore/views/import_export.py | 9 --------- .../views/tests/test_import_export.py | 7 +++---- lms/djangoapps/instructor/tests/test_api.py | 15 +++++++++------ lms/djangoapps/instructor_task/models.py | 12 ++---------- .../instructor_task/tests/test_models.py | 2 +- 5 files changed, 15 insertions(+), 30 deletions(-) diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index f80e0146f5..1e3d1487cc 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -27,7 +27,6 @@ from edx_django_utils.monitoring import set_custom_attribute, set_custom_attribu from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocator from path import Path as path -from storages.backends.s3boto import S3BotoStorage from storages.backends.s3boto3 import S3Boto3Storage from user_tasks.conf import settings as user_tasks_settings from user_tasks.models import UserTaskArtifact, UserTaskStatus @@ -381,14 +380,6 @@ def export_status_handler(request, course_key_string): artifact = UserTaskArtifact.objects.get(status=task_status, name='Output') if isinstance(artifact.file.storage, FileSystemStorage): output_url = reverse_course_url('export_output_handler', course_key) - elif isinstance(artifact.file.storage, S3BotoStorage): - filename = os.path.basename(artifact.file.name) - disposition = f'attachment; filename="{filename}"' - output_url = artifact.file.storage.url(artifact.file.name, response_headers={ - 'response-content-disposition': disposition, - 'response-content-encoding': 'application/octet-stream', - 'response-content-type': 'application/x-tgz' - }) elif isinstance(artifact.file.storage, S3Boto3Storage): filename = os.path.basename(artifact.file.name) disposition = f'attachment; filename="{filename}"' diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 7b6a68964e..d01f6800c1 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -25,7 +25,6 @@ from django.test.utils import override_settings from milestones.tests.utils import MilestonesTestCaseMixin from opaque_keys.edx.locator import LibraryLocator from path import Path as path -from storages.backends.s3boto import S3BotoStorage from storages.backends.s3boto3 import S3Boto3Storage from user_tasks.models import UserTaskStatus @@ -958,7 +957,7 @@ class ExportTestCase(CourseTestCase): """ Verify that the export status handler generates the correct export path for storage providers other than ``FileSystemStorage`` and - ``S3BotoStorage`` + ``S3Boto3Storage`` """ mock_latest_task_status.return_value = Mock(state=UserTaskStatus.SUCCEEDED) mock_get_user_task_artifact.return_value = self._mock_artifact( @@ -968,7 +967,7 @@ class ExportTestCase(CourseTestCase): result = json.loads(resp.content.decode('utf-8')) self.assertEqual(result['ExportOutput'], '/path/to/testfile.tar.gz') - @ddt.data(S3BotoStorage, S3Boto3Storage) + @ddt.data(S3Boto3Storage) @patch('cms.djangoapps.contentstore.views.import_export._latest_task_status') @patch('user_tasks.models.UserTaskArtifact.objects.get') def test_export_status_handler_s3( @@ -979,7 +978,7 @@ class ExportTestCase(CourseTestCase): ): """ Verify that the export status handler generates the correct export path - for the ``S3BotoStorage`` storage provider + for the ``S3Boto3Storage`` storage provider """ mock_latest_task_status.return_value = Mock(state=UserTaskStatus.SUCCEEDED) mock_get_user_task_artifact.return_value = self._mock_artifact( diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 15a6f06690..54adcb3e36 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -14,7 +14,6 @@ import dateutil import ddt import pytest import pytz -from boto.exception import BotoServerError from botocore.exceptions import ClientError from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user @@ -2747,10 +2746,14 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment """ Tests the Rate-Limit exceeded is handled and does not raise 500 error. """ - ex_status = 503 - ex_reason = 'Slow Down' url = reverse(endpoint, kwargs={'course_id': str(self.course.id)}) - with patch('storages.backends.s3boto.S3BotoStorage.listdir', side_effect=BotoServerError(ex_status, ex_reason)): + error_response = {'Error': {'Code': 503, 'Message': 'error found'}} + operation_name = 'test' + + with patch( + 'storages.backends.s3boto3.S3Boto3Storage.listdir', + side_effect=ClientError(error_response, operation_name) + ): if endpoint in INSTRUCTOR_GET_ENDPOINTS: response = self.client.get(url) else: @@ -2758,8 +2761,8 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment mock_error.assert_called_with( 'Fetching files failed for course: %s, status: %s, reason: %s', self.course.id, - ex_status, - ex_reason, + error_response.get('Error'), + error_response.get('Error').get('Message') ) res_json = json.loads(response.content.decode('utf-8')) diff --git a/lms/djangoapps/instructor_task/models.py b/lms/djangoapps/instructor_task/models.py index 2113af370d..1223ac3342 100644 --- a/lms/djangoapps/instructor_task/models.py +++ b/lms/djangoapps/instructor_task/models.py @@ -19,7 +19,6 @@ import logging import os.path from uuid import uuid4 -from boto.exception import BotoServerError from botocore.exceptions import ClientError from django.apps import apps from django.conf import settings @@ -41,7 +40,7 @@ QUEUING = 'QUEUING' PROGRESS = 'PROGRESS' SCHEDULED = 'SCHEDULED' TASK_INPUT_LENGTH = 10000 -DJANGO_STORE_STORAGE_CLASS = 'storages.backends.s3boto.S3BotoStorage' +DJANGO_STORE_STORAGE_CLASS = 'storages.backends.s3boto3.S3Boto3Storage' class InstructorTask(models.Model): @@ -332,14 +331,6 @@ class DjangoStorageReportStore(ReportStore): # Django's FileSystemStorage fails with an OSError if the course # dir does not exist; other storage types return an empty list. return [] - except BotoServerError as ex: - logger.error( - 'Fetching files failed for course: %s, status: %s, reason: %s', - course_id, - ex.status, - ex.reason - ) - return [] except ClientError as ex: logger.error( 'Fetching files failed for course: %s, status: %s, reason: %s', @@ -347,6 +338,7 @@ class DjangoStorageReportStore(ReportStore): ex.response.get('Error'), ex.response.get('Error', {}).get('Message') ) return [] + files = [(filename, os.path.join(course_dir, filename)) for filename in filenames] files.sort(key=lambda f: self.storage.get_modified_time(f[1]), reverse=True) return [ diff --git a/lms/djangoapps/instructor_task/tests/test_models.py b/lms/djangoapps/instructor_task/tests/test_models.py index 16e4e9486b..ff8e602b19 100644 --- a/lms/djangoapps/instructor_task/tests/test_models.py +++ b/lms/djangoapps/instructor_task/tests/test_models.py @@ -105,7 +105,7 @@ class DjangoStorageReportStoreS3TestCase(MockS3Boto3Mixin, ReportStoreTestMixin, storage. """ test_settings = copy.deepcopy(settings.GRADES_DOWNLOAD) - test_settings['STORAGE_CLASS'] = 'storages.backends.s3boto.S3BotoStorage' + test_settings['STORAGE_CLASS'] = 'storages.backends.s3boto3.S3Boto3Storage' test_settings['STORAGE_KWARGS'] = { 'bucket': settings.GRADES_DOWNLOAD['BUCKET'], 'location': settings.GRADES_DOWNLOAD['ROOT_PATH'],