Convert custom storage subclasses into mixins so that we can store to either the filesystem or S3

This commit is contained in:
Calen Pennington
2019-12-18 14:14:12 -05:00
parent 0ee1c2f05a
commit 151b309225
12 changed files with 62 additions and 39 deletions

View File

@@ -208,6 +208,12 @@ if 'loc_cache' not in CACHES:
if 'staticfiles' in CACHES:
CACHES['staticfiles']['KEY_PREFIX'] = EDX_PLATFORM_REVISION
# In order to transition from local disk asset storage to S3 backed asset storage,
# we need to run asset collection twice, once for local disk and once for S3.
# Once we have migrated to service assets off S3, then we can convert this back to
# managed by the yaml file contents
STATICFILES_STORAGE = os.environ.get('STATICFILES_STORAGE', ENV_TOKENS.get('STATICFILES_STORAGE', STATICFILES_STORAGE))
SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN')
SESSION_COOKIE_HTTPONLY = ENV_TOKENS.get('SESSION_COOKIE_HTTPONLY', True)
SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE)

View File

@@ -3297,7 +3297,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment
self.assertIn("attachment; filename=org", response['Content-Disposition'])
@patch('lms.djangoapps.instructor_task.models.logger.error')
@patch.dict(settings.GRADES_DOWNLOAD, {'STORAGE_TYPE': 's3'})
@patch.dict(settings.GRADES_DOWNLOAD, {'STORAGE_TYPE': 's3', 'ROOT_PATH': 'tmp/edx-s3/grades'})
def test_list_report_downloads_error(self, mock_error):
"""
Tests the Rate-Limit exceeded is handled and does not raise 500 error.

View File

@@ -84,7 +84,11 @@ class LocalFSReportStoreTestCase(ReportStoreTestMixin, TestReportMixin, SimpleTe
return ReportStore.from_config(config_name='GRADES_DOWNLOAD')
@patch.dict(settings.GRADES_DOWNLOAD, {'STORAGE_TYPE': 's3'})
@patch.dict(settings.GRADES_DOWNLOAD, {
'STORAGE_TYPE': 's3',
# Strip the leading `/`, because boto doesn't want it
'ROOT_PATH': settings.GRADES_DOWNLOAD['ROOT_PATH'].lstrip('/')
})
class S3ReportStoreTestCase(MockS3Mixin, ReportStoreTestMixin, TestReportMixin, SimpleTestCase):
"""
Test the old S3ReportStore configuration.

View File

@@ -244,6 +244,12 @@ if 'loc_cache' not in CACHES:
if 'staticfiles' in CACHES:
CACHES['staticfiles']['KEY_PREFIX'] = EDX_PLATFORM_REVISION
# In order to transition from local disk asset storage to S3 backed asset storage,
# we need to run asset collection twice, once for local disk and once for S3.
# Once we have migrated to service assets off S3, then we can convert this back to
# managed by the yaml file contents
STATICFILES_STORAGE = os.environ.get('STATICFILES_STORAGE', ENV_TOKENS.get('STATICFILES_STORAGE', STATICFILES_STORAGE))
# Email overrides
DEFAULT_FROM_EMAIL = ENV_TOKENS.get('DEFAULT_FROM_EMAIL', DEFAULT_FROM_EMAIL)
DEFAULT_FEEDBACK_EMAIL = ENV_TOKENS.get('DEFAULT_FEEDBACK_EMAIL', DEFAULT_FEEDBACK_EMAIL)

View File

@@ -47,7 +47,7 @@ class ThemeFilesFinder(BaseFinder):
themes = get_themes()
for theme in themes:
theme_storage = self.storage_class(
os.path.join(theme.path, self.source_dir),
location=os.path.join(theme.path, self.source_dir),
prefix=theme.theme_dir_name,
)

View File

@@ -28,7 +28,7 @@ from openedx.core.djangoapps.theming.helpers import (
)
class ThemeStorage(StaticFilesStorage):
class ThemeMixin(object):
"""
Comprehensive theme aware Static files storage.
"""
@@ -38,16 +38,10 @@ class ThemeStorage(StaticFilesStorage):
# instead of "images/logo.png"
prefix = None
def __init__(self, location=None, base_url=None, file_permissions_mode=None,
directory_permissions_mode=None, prefix=None):
def __init__(self, **kwargs):
self.prefix = prefix
super(ThemeStorage, self).__init__(
location=location,
base_url=base_url,
file_permissions_mode=file_permissions_mode,
directory_permissions_mode=directory_permissions_mode,
)
self.prefix = kwargs.pop('prefix', None)
super(ThemeMixin, self).__init__(**kwargs)
def url(self, name):
"""
@@ -76,7 +70,7 @@ class ThemeStorage(StaticFilesStorage):
if prefix and self.themed(name, prefix):
name = os.path.join(prefix, name)
return super(ThemeStorage, self).url(name)
return super(ThemeMixin, self).url(name)
def themed(self, name, theme):
"""
@@ -112,6 +106,10 @@ class ThemeStorage(StaticFilesStorage):
return self.exists(os.path.join(theme, name))
class ThemeStorage(ThemeMixin, StaticFilesStorage):
pass
class ThemeCachedFilesMixin(CachedFilesMixin):
"""
Comprehensive theme aware CachedFilesMixin.

View File

@@ -2,12 +2,12 @@
:class:`~django_require.staticstorage.OptimizedCachedRequireJsStorage`
"""
from openedx.core.storage import PipelineForgivingStorage
from openedx.core.storage import PipelineForgivingMixin
from pipeline.storage import PipelineCachedStorage
from require.storage import OptimizedFilesMixin
class OptimizedCachedRequireJsStorage(OptimizedFilesMixin, PipelineForgivingStorage):
class OptimizedCachedRequireJsStorage(OptimizedFilesMixin, PipelineForgivingMixin, PipelineCachedStorage):
"""
Custom storage backend that is used by Django-require.
"""

View File

@@ -7,19 +7,20 @@ from django.contrib.staticfiles.storage import StaticFilesStorage
from django.core.files.storage import get_storage_class, FileSystemStorage
from django.utils.deconstruct import deconstructible
from django.utils.lru_cache import lru_cache
from pipeline.storage import NonPackagingMixin, PipelineCachedStorage
from pipeline.storage import NonPackagingMixin
from require.storage import OptimizedFilesMixin
from storages.backends.s3boto3 import S3Boto3Storage
from openedx.core.djangoapps.theming.storage import ThemeCachedFilesMixin, ThemePipelineMixin, ThemeStorage
from openedx.core.djangoapps.theming.storage import ThemeCachedFilesMixin, ThemePipelineMixin, ThemeMixin
class PipelineForgivingStorage(PipelineCachedStorage):
class PipelineForgivingMixin(object):
"""
An extension of the django-pipeline storage backend which forgives missing files.
"""
def hashed_name(self, name, content=None, **kwargs):
try:
out = super(PipelineForgivingStorage, self).hashed_name(name, content, **kwargs)
out = super(PipelineForgivingMixin, self).hashed_name(name, content, **kwargs)
except ValueError:
# This means that a file could not be found, and normally this would
# cause a fatal error, which seems rather excessive given that
@@ -29,7 +30,7 @@ class PipelineForgivingStorage(PipelineCachedStorage):
def stored_name(self, name):
try:
out = super(PipelineForgivingStorage, self).stored_name(name)
out = super(PipelineForgivingMixin, self).stored_name(name)
except ValueError:
# This means that a file could not be found, and normally this would
# cause a fatal error, which seems rather excessive given that
@@ -38,25 +39,33 @@ class PipelineForgivingStorage(PipelineCachedStorage):
return out
class ProductionStorage(
PipelineForgivingStorage,
class ProductionMixin(
PipelineForgivingMixin,
OptimizedFilesMixin,
ThemePipelineMixin,
ThemeCachedFilesMixin,
ThemeStorage,
StaticFilesStorage
ThemeMixin,
):
"""
This class combines Django's StaticFilesStorage class with several mixins
that provide additional functionality. We use this version on production.
This class combines several mixins that provide additional functionality, and
can be applied over an existing Storage.
We use this version on production.
"""
pass
class ProductionStorage(ProductionMixin, StaticFilesStorage):
pass
class ProductionS3Storage(ProductionMixin, S3Boto3Storage): # pylint: disable=abstract-method
pass
class DevelopmentStorage(
NonPackagingMixin,
ThemePipelineMixin,
ThemeStorage,
ThemeMixin,
StaticFilesStorage
):
"""

View File

@@ -60,7 +60,7 @@ django-ses
django-simple-history
django-splash
django-statici18n==1.4.0
django-storages==1.4.1
django-storages
django-user-tasks
django-waffle==0.12.0
django-webpack-loader # Used to wire webpack bundles into the django asset pipeline

View File

@@ -81,7 +81,7 @@ django-ses==0.8.13
django-simple-history==2.8.0
django-splash==0.2.5
django-statici18n==1.4.0
django-storages==1.4.1
django-storages==1.8
django-user-tasks==0.3.0
django-waffle==0.12.0
django-webpack-loader==0.6.0
@@ -104,7 +104,7 @@ edx-django-release-util==0.3.2
edx-django-sites-extensions==2.4.2
edx-django-utils==2.0.2
edx-drf-extensions==2.4.5
edx-enterprise==2.0.36
edx-enterprise==2.0.37
edx-i18n-tools==0.5.0
edx-milestones==0.2.6
edx-oauth2-provider==1.3.1
@@ -175,7 +175,7 @@ paver==1.3.4
pbr==5.4.4
pdfminer.six==20191110
piexif==1.0.2
pillow==6.2.1
pillow==7.0.0
pkgconfig==1.5.1 # via xmlsec
polib==1.1.0 # via edx-i18n-tools
psutil==1.2.1

View File

@@ -94,7 +94,7 @@ django-ses==0.8.13
django-simple-history==2.8.0
django-splash==0.2.5
django-statici18n==1.4.0
django-storages==1.4.1
django-storages==1.8
django-user-tasks==0.3.0
django-waffle==0.12.0
django-webpack-loader==0.6.0
@@ -118,7 +118,7 @@ edx-django-release-util==0.3.2
edx-django-sites-extensions==2.4.2
edx-django-utils==2.0.2
edx-drf-extensions==2.4.5
edx-enterprise==2.0.36
edx-enterprise==2.0.37
edx-i18n-tools==0.5.0
edx-lint==1.3.0
edx-milestones==0.2.6
@@ -217,7 +217,7 @@ paver==1.3.4
pbr==5.4.4
pdfminer.six==20191110
piexif==1.0.2
pillow==6.2.1
pillow==7.0.0
pip-tools==4.3.0
pkgconfig==1.5.1
pluggy==0.13.1

View File

@@ -92,7 +92,7 @@ django-ses==0.8.13
django-simple-history==2.8.0
django-splash==0.2.5
django-statici18n==1.4.0
django-storages==1.4.1
django-storages==1.8
django-user-tasks==0.3.0
django-waffle==0.12.0
django-webpack-loader==0.6.0
@@ -115,7 +115,7 @@ edx-django-release-util==0.3.2
edx-django-sites-extensions==2.4.2
edx-django-utils==2.0.2
edx-drf-extensions==2.4.5
edx-enterprise==2.0.36
edx-enterprise==2.0.37
edx-i18n-tools==0.5.0
edx-lint==1.3.0
edx-milestones==0.2.6
@@ -209,7 +209,7 @@ paver==1.3.4
pbr==5.4.4
pdfminer.six==20191110
piexif==1.0.2
pillow==6.2.1
pillow==7.0.0
pkgconfig==1.5.1
pluggy==0.13.1
polib==1.1.0