-
Notifications
You must be signed in to change notification settings - Fork 6
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
drop offline caching from django-compressor to support compressing dynamic includes #401
Comments
If we can reasonably describe the steps to reproduce this issue, it should probably be filed with django-compressor upstream. |
AHA: http://django-compressor.readthedocs.org/en/latest/usage/#pre-compression explains what went on in #397. The form media for WYSIHTML5 was provided as a variable (namely, form media), and that variable (or an analog for it) is not specified in the I was thinking about this, and wondering how the offline compression command would populate the form variable in order to discover the media outside of the request/response cycle. The answer is: it doesn't. Unless you explicitly declare context for it to use when generating files for offline compression, the vars won't be defined. In short, Possible solutions:
Regarding the wysihtml5 / jsmin parsing: I think the simple solution is that if we put wysihtml5 assets in compress blocks, use the un-minified versions to avoid the crash. Whatever minification wysihtml5 is doing appears to root out a bug in jsmin. The minification we get from compress should do a good enough job. Alternatively, if we just keep wysihtml5 away from django-compressor, using the minified versions of the assets they ship should be fine. |
possible solution 4: switch away from offline compression in production. This would be essentially the same, except that the first request after the cache is cleared would be delayed while the server generated compressed assets. After that, performance would be roughly identical. This option gives us the most flexibility with least duplication. |
How would the fourth solution resolve the issue? Wouldn't compress still need to combine the static assets together, and still be unable to read the static assets due to variables? Or does it perform compression on-the-fly for each serve? That would seem to circumvent static hosting. So far I think the first option is working well and the third option might be something worth exploring; our README documentation is monolithic, so we just need to find a nice place in there to document how compressor works for this system. I think you nailed the problem though. I'm amazed that django-compressor doesn't have code to check for directives it cannot process, and instead naively tries to parse directives as though they were static assets. |
The way the fourth option works is:
So compress does need to still combine the static assets together; it just writes to s3 on page load (within the request/response cycle) instead of when the management command is run. As a result, it has access to the render context without having to duplicate that. If you did something silly like handing compress a different dynamic list of assets on each page load, compress would have to keep reconcatenating/minifying/etc. on every page load, and performance would drop drastically. But in this case, the list of assets is always the same unless the definition of the form class changes, so practically, every request beyond the first is the same and just hits the cache to determine the filename. |
I already like the fourth option because it doesn't require |
There is a potential problem with CloudFront/S3 latency. If the server pushes to S3, we might not see that result on CloudFront immediately. That's going to require CloudFront to reach back, pull forward, and cache the file on its side. Thankfully these files are uniquely named. |
Right: anything compress touches should have a hash of its contents as part of its filename. So if content changes a little bit (as long as we aren't ludicrously unlucky to stumble on a hash collision) it'll be a new filename. |
So if I understand this process, there would never be a need to run I think that is a big PRO to dropping offline. |
Alright, I'm going to rename this ticket to indicate we'd like to drop compressor offline support, with the goal of putting wyishtml5 back into the compress tags. This is a lower priority enhancement task since the current solution works fine and doesn't cause problems as-is. |
Follow up from #397 .
When wysihtml5 CSS or JS files are included in compressor and offline compression is enabled, it causes offline key errors which 500 crash the server in production mode. @yourcelf experienced TemplateSyntaxErrors using a system in dev mode.
The text was updated successfully, but these errors were encountered: