Opened 9 years ago

Closed 3 years ago

#24243 closed New feature (wontfix)

Allow HashedFilesMixin to handle file name fragments

Reported by: Laszlo Marai Owned by: Laszlo Marai
Component: contrib.staticfiles Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The hashed_name method will raise a ValueError if fed a non-existent file (including a file name fragment) which problematic for several reasons:

  • File name fragments can't be used this way (e.g. I want to do {% url '/static/sounds/some_sound' %} and then append the extension from javascript)
  • The above technique works with cache middle ware not using HashedFilesMixin, so the ValueError comes kind of unexpected (e.g. when only using it on staging/deployment environments and not locally)
  • Actually just below the offending code it explicitly tries to handle fragments, even has a comment about it, but the exception is raised before.

Problematic code starts on line 89.

Change History (18)

comment:1 by Laszlo Marai, 9 years ago

Owner: changed from nobody to Laszlo Marai
Status: newassigned

comment:2 by Laszlo Marai, 9 years ago

Owner: Laszlo Marai removed
Status: assignednew

comment:3 by Laszlo Marai, 9 years ago

After looking into it, now I see that I misunderstood the word 'fragment' in the comments and it refers to url fragments, not file name fragments (as in partial file names). However, what I have described above is still a valid behaviour and in line with the default django.contrib.staticfiles.storage.StaticFilesStorage behaviour.

comment:4 by Laszlo Marai, 9 years ago

Owner: set to Laszlo Marai
Status: newassigned

comment:5 by Tim Graham, 9 years ago

Easy pickings: unset

Can you offer a patch for this issue? I'm having trouble understanding exactly what changes you're proposing to make?

comment:6 by Tim Graham, 9 years ago

Resolution: needsinfo
Status: assignedclosed

comment:7 by Laszlo Marai, 9 years ago

The change I'm suggesting is that resolving a non-existent file name doesn't raise a ValueError when using ManifestStaticFilesStorage (change needed in HashedFilesMixin). Though it was 5 weeks ago, it seems I have done the fix back then.

Do I attach a file containing the suggested fix here or do I create a pull request? (The documentation wasn't 100% clear.)

comment:8 by Tim Graham, 9 years ago

Pull requests are preferred. Let me know which documentation you were confused by so we can update it.

comment:9 by Laszlo Marai, 9 years ago

Pull request created. Can't remember the documentation that caused the confusion, it was over a month ago. Will let you know if I see it again.

comment:10 by Aymeric Augustin, 9 years ago

Isn't this a duplicate of #18958?

comment:11 by Laszlo Marai, 9 years ago

Nope, it isn't. What I'm doing is being able to use

{% static 'wherever/some/of/your/files/are/' %}

or e.g. (generated js)

var sound = '{% static 'sounds/mysound' %}';

and then append .mp3 or .ogg depending on the browser capabilities. Both files are present. Actually #18958 would still be an issue for the reporter after accepting my patch.

comment:12 by Tim Graham, 9 years ago

Resolution: needsinfo
Status: closednew
Type: UncategorizedNew feature

Reopening for further consideration.

comment:13 by Aymeric Augustin, 9 years ago

With the example above, var sound = '{% static 'sounds/mysound' %}'; should be rendered as var sound = '/static/sounds/mysound.e678b76a';. How does Django determine the hash to insert there?

comment:14 by Laszlo Marai, 9 years ago

It does not. In this case, cache busting doesn't work.

comment:15 by Tim Graham, 9 years ago

Has patch: set
Patch needs improvement: set
Summary: HashedFilesMixin doesn't handle file name fragments (thus the url template tag neither)Allow HashedFilesMixin to handle file name fragments
Triage Stage: UnreviewedAccepted
Version: 1.7master

I'm tentatively accepting this ticket, although I'd like to confirm with @jezdez who authored HashedFilesMixin.

As far as I could tell when using it (I tested with CachedStaticFilesStorage), {% static 'foo/bar' %} (that doesn't exist) will only cause an error when DEBUG=False (without this patch). This doesn't seem like a great behavior (errors go undetected until you're running in production). A better behavior might be log a warning when DEBUG=True. With this patch, references to missing files go undetected, but at least won't get crashes in production for missing files.

comment:16 by Ed Johnson, 8 years ago

Hi all,

What happened to this bug? It's killing my deployment to production on Heroku with DEBUG=False. Raising an uncaught ValueError in post_process that kills collectstatic and breaks deployments is a sub optimal design decision.

A single missing static file blocks my entire production deployment for Django 1.7 - 1.9

I should be able to override this behavior with something like an optional --no-strict flag on collectstatic.
The default could be strict (as it is today), and then developers like me could make the choice to get our deployment out to production with a missing file, instead of being completely (and surprisingly) blocked.

What are the steps to re-open this?

comment:17 by Tim Graham, 8 years ago

The ticket is open. It's waiting for a patch.

comment:18 by Mariusz Felisiak, 3 years ago

Has patch: unset
Patch needs improvement: unset
Resolution: wontfix
Status: newclosed
Triage Stage: AcceptedUnreviewed

I don't think it's worth the potential confusion and if someone wants to ignore the missing files they can always use a ManifestStaticFilesStorage subclass (see David's comment). There is no need to add this to Django itself.

Note: See TracTickets for help on using tickets.
Back to Top