Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Update from boto to boto3 storage backend. #31759

Merged
merged 10 commits into from
Feb 21, 2023
3 changes: 1 addition & 2 deletions cms/djangoapps/cms_user_tasks/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"""
import json

from boto.exception import NoAuthHandlerFound
from celery import shared_task
from celery.exceptions import MaxRetriesExceededError
from celery.utils.log import get_task_logger
Expand Down Expand Up @@ -52,7 +51,7 @@ def send_task_complete_email(self, task_name, task_state_text, dest_addr, detail
try:
mail.send_mail(subject, message, from_address, [dest_addr], fail_silently=False)
LOGGER.info("Task complete email has been sent to User %s", dest_addr)
except NoAuthHandlerFound:
except Exception:
LOGGER.info(
'Retrying sending email to user %s, attempt # %s of %s',
dest_addr,
Expand Down
6 changes: 3 additions & 3 deletions cms/djangoapps/cms_user_tasks/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from uuid import uuid4

import ddt
from boto.exception import NoAuthHandlerFound
from django.conf import settings
from django.core import mail
from django.test import override_settings
Expand Down Expand Up @@ -302,8 +301,9 @@ def test_email_retries(self):
"""
Make sure we can succeed on retries
"""

with mock.patch('django.core.mail.send_mail') as mock_exception:
mock_exception.side_effect = NoAuthHandlerFound()
mock_exception.side_effect=Exception()

with mock.patch('cms.djangoapps.cms_user_tasks.tasks.send_task_complete_email.retry') as mock_retry:
user_task_stopped.send(sender=UserTaskStatus, status=self.status)
Expand All @@ -315,7 +315,7 @@ def test_queue_email_failure(self):
logger.addHandler(hdlr)

with mock.patch('cms.djangoapps.cms_user_tasks.tasks.send_task_complete_email.delay') as mock_delay:
mock_delay.side_effect = NoAuthHandlerFound()
mock_delay.side_effect = Exception()
user_task_stopped.send(sender=UserTaskStatus, status=self.status)
self.assertTrue(mock_delay.called)
self.assertEqual(hdlr.messages['error'][0], 'Unable to queue send_task_complete_email')
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@

from django.conf import settings
from django.core.files.storage import get_storage_class
from storages.backends.s3boto import S3BotoStorage
from storages.backends.s3boto3 import S3Boto3Storage
from storages.utils import setting


class ImportExportS3Storage(S3BotoStorage): # pylint: disable=abstract-method
class ImportExportS3Storage(S3Boto3Storage): # pylint: disable=abstract-method
"""
S3 backend for course import and export OLX files.
"""
Expand Down
4 changes: 2 additions & 2 deletions cms/djangoapps/export_course_metadata/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@

from django.conf import settings
from django.core.files.storage import get_storage_class
from storages.backends.s3boto import S3BotoStorage
from storages.backends.s3boto3 import S3Boto3Storage


class CourseMetadataExportS3Storage(S3BotoStorage): # pylint: disable=abstract-method
class CourseMetadataExportS3Storage(S3Boto3Storage): # pylint: disable=abstract-method
"""
S3 backend for course metadata export
"""
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/export_course_metadata/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ def export_course_metadata_task(self, course_key_string): # pylint: disable=unu
"""
course_key = CourseKey.from_string(course_key_string)
highlights = get_all_course_highlights(course_key)
highlights_content = ContentFile(json.dumps({'highlights': highlights}))
highlights_content = ContentFile(json.dumps({'highlights': highlights}).encode('utf-8'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, this is apparently an open bug in django-storages with a PR to fix it that's been sitting around unmerged for almost 3 years for no particularly good reason: jschneier/django-storages#911

course_metadata_export_storage.save(f'course_metadata_export/{course_key}.json', highlights_content)
2 changes: 1 addition & 1 deletion cms/djangoapps/export_course_metadata/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_happy_path(self, patched_content, patched_storage):
SignalHandler.course_published.connect(export_course_metadata)
SignalHandler.course_published.send(sender=None, course_key=self.course_key)
patched_content.assert_called_once_with(
'{"highlights": [["week1highlight1", "week1highlight2"], ["week1highlight1", "week1highlight2"], [], []]}'
b'{"highlights": [["week1highlight1", "week1highlight2"], ["week1highlight1", "week1highlight2"], [], []]}'
)
patched_storage.save.assert_called_once_with(
f'course_metadata_export/{self.course_key}.json', patched_content.return_value
Expand Down