Code

Opened 3 years ago

Closed 3 years ago

#15252 closed New feature (fixed)

get_static_url in static templatetags

Reported by: ohardy Owned by: nobody
Component: contrib.staticfiles Version: master
Severity: Normal Keywords:
Cc: streeter Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Hello,

When using the settings STATICFILES_STORAGE, it's the storage that should return the url.

For example, a tag of this type works very well:

@register.simple_tag()
def get_static_url(filepath):
    if storage.exists(filepath):
        return storage.url(filepath)
    return None

and so STATIC_URL is not required anymore

Attachments (5)

15252.1.diff (2.3 KB) - added by jezdez 3 years ago.
initial patch
15252.2.diff (12.5 KB) - added by jezdez 3 years ago.
Adds working hashed file storage backend
15252.2.2.diff (11.8 KB) - added by jezdez 3 years ago.
Adds working hashed file storage backend
15252.3.diff (40.3 KB) - added by jezdez 3 years ago.
Latest state of the Github repo at https://github.com/jezdez/django/compare/feature/staticfiles-templatetag
15252.4.diff (60.3 KB) - added by jezdez 3 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 3 years ago by ohardy

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Forget that before:

from django import template
from django.core.files.storage import get_storage_class

register = template.Library()

storage = get_storage_class(settings.STATICFILES_STORAGE)()

comment:2 Changed 3 years ago by jezdez

  • Component changed from Uncategorized to django.contrib.staticfiles
  • Triage Stage changed from Unreviewed to Design decision needed

I'm not sure how that would be an improvement to the get_static_prefix template tag, to be honest. Would you mind to elaborate?

http://docs.djangoproject.com/en/dev/ref/contrib/staticfiles/#the-get-static-prefix-templatetag

comment:3 Changed 3 years ago by ohardy

Several differences:

  • STATIC_URL is not required since the storage is constructing the URL
  • If a storage wants to build a URL encoded, for example, it is impossible to build this URL with STATIC_URL.
  • STATIC_URL been given duplicate such that for S3 storage as we already set the URL when you declare storage.

All these remarks also apply to MEDIA_URL

comment:4 Changed 3 years ago by jezdez

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Design decision needed to Accepted

Having given this a bit more thought, I'm convinced this is a useful. There are a few edge cases though:

  1. The docs need to carefully explain how the storage influences the result of this template tag, e.g. signed URLs, increased latency with remote storage backends etc. It's usually also a pretty advanced topic, so we need to make sure this won't confuse beginners.
  1. FileFields and ImageFields can be passed different storage backends, which is why the template tag doesn't completely apply to MEDIA_URL. It should probably check whether it's passed a file instance or a string, e.g.
@register.simple_tag()
def get_media_url(file_or_path):
    if isinstance(file_or_path, UploadedFile):
        return file_or_path.url
    return storage.url(file_or_path)

comment:5 Changed 3 years ago by lrekucki

  • Severity set to Normal
  • Type set to New feature

Changed 3 years ago by jezdez

initial patch

comment:6 Changed 3 years ago by jezdez

  • Easy pickings set
  • Has patch set
  • Needs tests unset
  • Patch needs improvement unset
  • UI/UX unset

The attached patch implements this idea solely for staticfiles since I don't believe it's really needed for MEDIA_URL (and the file storage backends) -- the filefields are capable of returning the url themeselves.

Changed 3 years ago by jezdez

Adds working hashed file storage backend

Changed 3 years ago by jezdez

Adds working hashed file storage backend

comment:7 Changed 3 years ago by jezdez

Sorry for the double upload, I just updated the patch to be against the latest trunk.

Changed 3 years ago by jezdez

comment:8 Changed 3 years ago by jezdez

  • Needs documentation unset

comment:9 Changed 3 years ago by streeter

  • Cc streeter added

Changed 3 years ago by jezdez

comment:10 Changed 3 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

In [16594]:

Fixed #15252 -- Added static template tag and CachedStaticFilesStorage to staticfiles contrib app.

Many thanks to Florian Apolloner and Jacob Kaplan-Moss for reviewing and eagle eyeing.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.