Enforce static file look up for ProductionS3Storage backend #907
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OmarIthawi
commented
May 8, 2021
| @@ -32,7 +33,8 @@ | |||
|
|
|||
| <%def name='url(file, raw=False)'><% | |||
| try: | |||
| url = staticfiles_storage.url(file) | |||
| with beeline.tracer(name='static_content.html.url'): | |||
There was a problem hiding this comment.
Debugging static files lookup.
| @@ -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): | |||
There was a problem hiding this comment.
Debugs rendering templates, which includes all/most calls to static files lookups.
OmarIthawi
force-pushed
the
omar/honeycomb-staticfiles
branch
from
f05af7a
to
f1c6cc1
Compare
thraxil
approved these changes
May 8, 2021
OmarIthawi
force-pushed
the
omar/honeycomb-staticfiles
branch
from
86eda1c
to
f706cbb
Compare
OmarIthawi
changed the title
…ry deploy and use the random prefix Related to appsembler/configuration#348
this mostly to debug slowness in static files look up as reported in RED-1961.
settings.CACHES['staticfiles'] wasn't getting hit when ProductionS3Storage is used in HTTP request context. This change enforces cache by overriding the ProductionS3Storage.url method.
OmarIthawi
force-pushed
the
omar/honeycomb-staticfiles
branch
from
c8a1fe9
to
9e1e8d0
Compare
thraxil
approved these changes
May 12, 2021
There was a problem hiding this comment.
I'm in favor of getting this out quickly to unblock Juniper.
Longer term, we probably want to consider some things:
- instead of adding our stuff directly to
openedx.core.storage.ProductionS3Storage, we probably ought to make our own storage class that just subclasses it to overrideurl()and add the explicit caching, then useAppsemblerProductionS3Storagein our config. - having honeycomb tracing on every call to
url()is useful at the moment for debugging/verifying this fix, but is probably more verbose than we're going to want to dial it down later. - instead of commenting out the setting of the key prefix here and putting a random value in the config in Increase edxapp staticfiles cache timeout to 30 days to improve performance configuration#361 maybe we could instead set
EDX_PLATFORM_REVISIONto a git hash or cloud build revision id during the deploy (with something like an ansible--extra-vars=EDX_PLATFORM_REVISION=$REVISION_IDin the deploy script. That would achieve the same effect of making sure that the cache is cleared on each deploy.
johnbaldwin
approved these changes
May 12, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RED-1961.
CACHES['staticfiles']['KEY_PREFIX'] = EDX_PLATFORM_REVISIONby upstream which assumes we're deploying via git tags. We don't, we deploy frommainand the upstream override keeps the cache stale and invalid.ProductionS3Storage.urllook up results manually.Tests