diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 8822b6aac7..15a6f06690 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -15,6 +15,7 @@ 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 from django.core import mail @@ -2742,7 +2743,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment @patch('lms.djangoapps.instructor_task.models.logger.error') @patch.dict(settings.GRADES_DOWNLOAD, {'STORAGE_TYPE': 's3', 'ROOT_PATH': 'tmp/edx-s3/grades'}) @ddt.data('list_report_downloads', 'instructor_api_v1:list_report_downloads') - def test_list_report_downloads_error(self, endpoint, mock_error): + def test_list_report_downloads_error_boto(self, endpoint, mock_error): """ Tests the Rate-Limit exceeded is handled and does not raise 500 error. """ @@ -2764,6 +2765,39 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment res_json = json.loads(response.content.decode('utf-8')) assert res_json == {'downloads': []} + @patch('lms.djangoapps.instructor_task.models.logger.error') + @patch( + 'lms.djangoapps.instructor_task.models.DJANGO_STORE_STORAGE_CLASS', + 'storages.backends.s3boto3.S3Boto3Storage' + ) + @patch.dict(settings.GRADES_DOWNLOAD, {'STORAGE_TYPE': 's3', 'ROOT_PATH': 'tmp/edx-s3/grades'}) + @ddt.data('list_report_downloads', 'instructor_api_v1:list_report_downloads') + def test_list_report_downloads_error_boto3(self, endpoint, mock_error): + """ + Tests the Rate-Limit exceeded is handled and does not raise 500 error. + """ + error_response = {'Error': {'Code': 503, 'Message': 'error found'}} + operation_name = 'test' + url = reverse(endpoint, kwargs={'course_id': str(self.course.id)}) + 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: + response = self.client.post(url, {}) + + mock_error.assert_called_with( + 'Fetching files failed for course: %s, status: %s, reason: %s', + self.course.id, + error_response.get('Error'), + error_response.get('Error').get('Message') + ) + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json == {'downloads': []} + @ddt.data('list_report_downloads', 'instructor_api_v1:list_report_downloads') def test_list_report_downloads(self, endpoint): url = reverse(endpoint, kwargs={'course_id': str(self.course.id)}) diff --git a/lms/djangoapps/instructor_task/models.py b/lms/djangoapps/instructor_task/models.py index 3655bccf17..2113af370d 100644 --- a/lms/djangoapps/instructor_task/models.py +++ b/lms/djangoapps/instructor_task/models.py @@ -20,6 +20,7 @@ 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 from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user @@ -40,6 +41,7 @@ QUEUING = 'QUEUING' PROGRESS = 'PROGRESS' SCHEDULED = 'SCHEDULED' TASK_INPUT_LENGTH = 10000 +DJANGO_STORE_STORAGE_CLASS = 'storages.backends.s3boto.S3BotoStorage' class InstructorTask(models.Model): @@ -230,7 +232,7 @@ class ReportStore: storage_type = config.get('STORAGE_TYPE', '').lower() if storage_type == 's3': return DjangoStorageReportStore( - storage_class='storages.backends.s3boto.S3BotoStorage', + storage_class=DJANGO_STORE_STORAGE_CLASS, storage_kwargs={ 'bucket': config['BUCKET'], 'location': config['ROOT_PATH'], @@ -265,6 +267,7 @@ class DjangoStorageReportStore(ReportStore): def __init__(self, storage_class=None, storage_kwargs=None): if storage_kwargs is None: storage_kwargs = {} + self.storage = get_storage(storage_class, **storage_kwargs) @classmethod @@ -337,6 +340,13 @@ class DjangoStorageReportStore(ReportStore): ex.reason ) return [] + except ClientError as ex: + logger.error( + 'Fetching files failed for course: %s, status: %s, reason: %s', + course_id, + 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 [