CachedStaticFilesStorage should handle errors with broken CSS

[yxven]

I am trying to deploy my website. My website makes heavy use of the Dojo Javascript Toolkit. If you're not familiar with it, dojo is a massive javascript library with thousands of files. Every file is not production ready. There are experimental sub-libraries.

When I try to collectstatic this library, it constantly crashes at broken css links to images that don't exist. These are not broken links I want to fix. These are links that are only required by broken experimental plugins to experimental classes that I am not using. This adds to my workload because I now have to go through the library deleting anything I don't need.

What happens in the future when I decide to use one of these classes? If I'm lucky I can figure out exactly what I need and replace it, but in reality, I'm going to have to recopy the library and hack out everything I don't need. Alternatively, I can use code repository tricks to try to replace only the code I needed.

What if I can't just delete the css file? What if I'm using parts of it elsewhere? I'd probably manually edit out the offending line. What if I need the widget that line referred to in the future? It's now going to fail silently. Unless it's completely obvious, I'm not going to notice because I've never used that widget before.

What CachedStaticFilesStorage should do is blindly copy the files while mangling the names with the md5 hash. If there's a broken css link, it's not CachedStaticFileStorage's problem. Alternately, it should warn the user with a dialogue to proceed anyway instead of crashing.

[Aymeric Augustin]

blindly copy the files while mangling the names with the md5 hash

It is rather difficult to compute the md5 hash of a file when the file doesn't exist. By design CachedStaticFilesStorage isn't usable with CSS that's broken in this way.

However, a better error message pointing to the offending source file and missing target file would help.

[yxven]

I didn't realize CachedFileStorage was modifying the css to fix the links. I understand that must be necessary for CachedFileStorage to function.

That said, I was unable to triage my dojo installation by deleting css files. There were too many dependencies.

It should just warn you that the link will be broken. I should be able to tell it to ignore the broken link and keep collecting.

[Jannis Leidel]

No, it shouldn't just warn that the links is broken.

It was a distinct design decision to only cache files that refer to other files correctly. If the sub-libraries are so experimental that images are missing I would call them broken and not supposed to be deployed by the cached storage backend in the first place.

[Matthew Tretter]

IMO this design decision is misguided. Yes, the CSS files are broken in the sense that they contain broken links, however broken links don't make for invalid CSS. It doesn't seem right to have what's essentially an optimization step place stricter requirements on the source than the language itself.

Practically speaking, given how little browsers complain, this kind of error is very easy to miss—and therefore very likely to make it into (even popular) third-party libraries. (I ran into this issue with django-cms.) Yes, this is an error in the CSS file, but the current behavior of CachedStaticFilesStorage changes the nature and severity of that error.

[Aymeric Augustin]

Yes, I've also encountered the problem with django-cms before. I just submitted a pull request: ​https://github.com/divio/django-cms/pull/1508

[anonymous]

I agree it's misguided not to "fix" CachedStaticFilesStorage in this case.

The decision to be "pure" and require totally clean CSS renders CachedStaticFilesStorage useless. I don't have time to sift through issues in 3rd party libraries right now. So I'm disabling CachedStaticFilesStorage. Which is detrimental to my site as a whole.

Wouldn't the greater good be accomplished by making CachedStaticFilesStorage more permissive?

It's not ideal, but the zen of Pythons says "practicality beats purity". I think this is a case where that principle applies.

[Aymeric Augustin]

"Greater good" sounds cool, but that doesn't make it possible to compute the MD5 hash of a file that doesn't exist!

[Aymeric Augustin]

See #18986 for a follow-up on this issue.

[andrewsg]

aaugustin, do you think a patch that allowed the user to tell collectstatic (for instance via a command-line --argument) to turn these errors into warnings and just move on, leaving the bad relative URL in place as-is, would be acceptable?

[Aymeric Augustin]

Maybe :)

You could test the waters on the django-developers mailing list.

[anonymous]

This comes up all the time in massive libraries like boostrap, jquery-ui, etc... I see no reason or harm done in producing a warning that the file is missing and move on the the next file. The argument, "but that doesn't make it possible to compute the MD5 hash of a file that doesn't exist!" is absurd!!! CLEARLY IF THE FILE DOES NOT EXIST A WARING THAT THE FILE IS MISSING IS PRODUCED INSTEAD OF COMPUTING THE MD5! You know, something like try: apply an MD5 hash, except: print a warning that the file is missing. Further more, I think most people have taken the approach 'So I'm disabling CachedStaticFilesStorage' and find other solutions like django-compressor. The fact is, not enough people are using CachedStaticFilesStorage to care or complain about it, because of this issue. The way it is, django should just depreciate and remove this utterly useless feature or enter a little try/excep and we have a useful built in solution.

[Aymeric Augustin]

You seemed to have missed comment 10 that makes exactly the same point, but politely.

If you feel so strongly about this, write a patch. I don't care much, because my CSS is generally correct.