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

Enforce static file look up for ProductionS3Storage backend #907

Merged
merged 3 commits into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions cms/envs/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,11 @@ def get_env_setting(setting):
'LOCATION': 'edx_location_mem_cache',
}

if 'staticfiles' in CACHES:
CACHES['staticfiles']['KEY_PREFIX'] = EDX_PLATFORM_REVISION
# Tahoe: RED-1961 Disable the upstream prefix override to refresh cache on every deploy and use the random prefix:
# https://github.com/appsembler/configuration/pull/348
#
# 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.
Expand Down
4 changes: 4 additions & 0 deletions common/djangoapps/edxmako/shortcuts.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import logging

import beeline
import six
from django.conf import settings
from django.http import HttpResponse
Expand Down Expand Up @@ -149,6 +150,7 @@ def marketing_link_context_processor(request):
)


@beeline.traced(name='common.djangoapps.edxmako.shortcuts.render_to_string')
def render_to_string(template_name, dictionary, namespace='main', request=None):
Copy link
Author

Choose a reason for hiding this comment

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

Debugs rendering templates, which includes all/most calls to static files lookups.

"""
Render a Mako template to as a string.
Expand All @@ -169,6 +171,8 @@ def render_to_string(template_name, dictionary, namespace='main', request=None):
request: The request to use to construct the RequestContext for rendering
this template. If not supplied, the current request will be used.
"""
beeline.add_context_field('render_to_string.template_name', template_name)
beeline.add_context(dictionary)
if namespace == 'lms.main':
engine = engines[Engines.PREVIEW]
else:
Expand Down
14 changes: 10 additions & 4 deletions common/djangoapps/pipeline_mako/templates/static_content.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<%!
import logging
import json
import beeline
from django.contrib.staticfiles.storage import staticfiles_storage
from pipeline_mako import compressed_css, compressed_js
from pipeline_mako.helpers.studiofrontend import load_sfe_i18n_messages
Expand Down Expand Up @@ -31,10 +32,15 @@
%></%def>

<%def name='url(file, raw=False)'><%
try:
url = staticfiles_storage.url(file)
except:
url = file
with beeline.tracer(name='static_content.html.url'):
beeline.add_context_field('url.file', file)
try:
url = staticfiles_storage.url(file)
beeline.add_context_field('url.error', False)
except:
beeline.add_context_field('url.error', True)
url = file
beeline.add_context_field('url.url', url)
## HTML-escaping must be handled by caller
%>${url | n, decode.utf8}${"?raw" if raw else ""}</%def>

Expand Down
7 changes: 5 additions & 2 deletions lms/envs/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,11 @@ def get_env_setting(setting):
'LOCATION': 'edx_location_mem_cache',
}

if 'staticfiles' in CACHES:
CACHES['staticfiles']['KEY_PREFIX'] = EDX_PLATFORM_REVISION
# Tahoe: RED-1961 Disable the upstream prefix override to refresh cache on every deploy and use the random prefix:
# https://github.com/appsembler/configuration/pull/348
#
# 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.
Expand Down
28 changes: 27 additions & 1 deletion openedx/core/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
Django storage backends for Open edX.
"""

import beeline

from django.conf import settings
from django.contrib.staticfiles.storage import StaticFilesStorage
from django.core.files.storage import get_storage_class, FileSystemStorage
from django.core.cache import caches
from django.utils.deconstruct import deconstructible
from django.utils.lru_cache import lru_cache
from pipeline.storage import NonPackagingMixin
Expand Down Expand Up @@ -62,7 +64,31 @@ class ProductionStorage(ProductionMixin, StaticFilesStorage):


class ProductionS3Storage(ProductionMixin, S3Boto3Storage): # pylint: disable=abstract-method
pass

@beeline.traced('ProductionS3Storage.url')
def url(self, name, force=False):
"""
Return the non-hashed URL in DEBUG mode with cache support.

Tahoe: RED-1961 This method is created for Tahoe to address mysteriously missing cache.
"""
beeline.add_context_field('ProductionS3Storage.file_name', name)
beeline.add_context_field('ProductionS3Storage.force', force)

static_files_cache = caches['staticfiles']
cache_entry_name = 'ProductionS3Storage.staticfiles_cache.{}'.format(name)
beeline.add_context_field('ProductionS3Storage.staticfiles_cache', cache_entry_name)

url = static_files_cache.get(cache_entry_name)
if not url:
beeline.add_context_field('ProductionS3Storage.cached', False)
url = super().url(name, force)
static_files_cache.set(cache_entry_name, url)
else:
beeline.add_context_field('ProductionS3Storage.cached', True)

beeline.add_context_field('ProductionS3Storage.hashed_url', url)
return url


class DevelopmentStorage(
Expand Down