Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#15270 closed (fixed)

augment staticfiles_urlpatterns to accept kwarg 'document_root'

Reported by: Mike Fogel Owned by: Jannis Leidel
Component: contrib.staticfiles Version: master
Severity: Keywords:
Cc: Mike Fogel Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

I want to be able to trail my urls.py with

urlpatterns += staticfiles_urlpatterns()
urlpatterns += staticfiles_urlpatterns(prefix=settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)

Rather than the currently suggested best practice from the documentation (http://docs.djangoproject.com/en/dev/ref/contrib/staticfiles/#serving-other-directories):

  urlpatterns += staticfiles_urlpatterns()
  if settings.DEBUG:
      urlpatterns += patterns('django.contrib.staticfiles.views',
           url(r'^media/(?P<path>.*)$', 'serve',
               {'document_root': settings.MEDIA_ROOT}), )

If this is agreed to in principle I'll write up a short patch and submit it. Thanks.

Attachments (1)

15270.1.diff (17.4 KB) - added by Jannis Leidel 6 years ago.
moves back the code, adds static url pattern helper that is used by staticfiles now, too. probably needs more doc updates in the howto section

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

I agree that the "deploy Media URLs under debug" pattern is extraordinarily common, and should be supported in much the same way that the automated staticfiles serving is supported.

I'm not necessarily convinced that the way you propose to do this is the right way, though -- if only because one of the general principles of contrib is that we should be able to delete the contrib directory and Django should still operate. Staticfiles is an optional component, so it doesn't seem right that media serving, which is part of the core framework, should be serviced by an optional component. If anything, the dependency should be the other way around.

Also -- you should note that you don't have to put staticfiles_urlpatterns() in your urls.py at all. The development server will automatically serve those urls. A manual staticfiles_urlpatterns() definition is only needed if you're using a development server other than Django's.

comment:2 Changed 6 years ago by Mike Fogel

I didn't realize the development server automatically served the staticfiles urls if contrib.staticfiles is in INSTALLED_APPS, thanks. I guess then my feeling is that if static files are served automatically by development server, then media files probably should be too.

I'm all for not establishing dependencies from core out to contrib, but it looks like views.static.serve already is already just a wrapper around contrib.staticfiles.serve (source:django/trunk/django/views/static.py#L20) There are also dependencies from the development server to contrib.staticfiles in order to serve admin media (source:django/trunk/django/core/servers/basehttp.py). So this wouldn't be establishing a new dependency, it would be more making better use of an existing one.

Is there a way to serve media in the developement server without using views.static.serve or contrib.staticfiles?

comment:3 in reply to:  2 Changed 6 years ago by Jannis Leidel

Replying to carbonXT:

I didn't realize the development server automatically served the staticfiles urls if contrib.staticfiles is in INSTALLED_APPS, thanks. I guess then my feeling is that if static files are served automatically by development server, then media files probably should be too.

I disagree strongly, staticfiles' automatic serving is limited to its finder API, so adding another "media files" serving ability to staticfiles runserver (or core's) is a no-go.

Here's what you can during development: add MEDIA_ROOT to the locations where staticfiles looks for files:

STATIC_URL = '/static/'
STATIC_ROOT = os.path.join(PROJECT_ROOT, 'static')

MEDIA_URL = '/static/media/'
MEDIA_ROOT = os.path.join(PROJECT_ROOT, 'media')

if DEBUG:
    STATICFILES_DIRS += (
        ('media', MEDIA_ROOT),
    )

Then, in production you will need your webserver to alias /static/media/ to MEDIA_ROOT and /static/ to STATIC_ROOT, in that order.

Note, of course this also means that when you call collectstatic staticfiles happily collects files from MEDIA_ROOT and puts them under STATIC_ROOT/media. So you probably only want to add MEDIA_ROOT to STATICFILES_DIRS when in dev mode or you're okay with duplicate files.

I'm all for not establishing dependencies from core out to contrib, but it looks like views.static.serve already is already just a wrapper around contrib.staticfiles.serve (source:django/trunk/django/views/static.py#L20) There are also dependencies from the development server to contrib.staticfiles in order to serve admin media (source:django/trunk/django/core/servers/basehttp.py). So this wouldn't be establishing a new dependency, it would be more making better use of an existing one.

Well, django.views.static was moved from core to staticfiles since it didn't make sense to have it there if a special contrib app exists, we've done this before. So this new "dependency" is due to the fact django.views.static is now pending deprecation, just like the AdminMediaHandler in django.core.servers.basehttp.

Is there a way to serve media in the developement server without using views.static.serve or contrib.staticfiles?

No.

comment:4 Changed 6 years ago by Jannis Leidel

Owner: set to Jannis Leidel

Okay, having talked to Carl on IRC a bit about this and the recent #15199, I think we now have a good plan to get to the bottom of this problem: static view being part of staticfiles. I'm going to try moving it back and provide a URL pattern helper for MEDIA_* serving along the lines of staticfiles' staticfiles_urlpatterns helper.

comment:5 Changed 6 years ago by Jannis Leidel

milestone: 1.41.3

Changed 6 years ago by Jannis Leidel

Attachment: 15270.1.diff added

moves back the code, adds static url pattern helper that is used by staticfiles now, too. probably needs more doc updates in the howto section

comment:6 Changed 6 years ago by Jannis Leidel

Further updates can be found here: https://github.com/jezdez/django/commits/15270

comment:7 Changed 6 years ago by Jannis Leidel

Has patch: set
Needs documentation: set
Needs tests: set

comment:8 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [15530]:

Fixes #15270 -- Moved back the serve view to django.views.static due to dependency conflicts with the contrib app staticfiles (reverts parts of r14293). Added a helper function that generates URL patterns for serving static and media files during development. Thanks to Carl for reviewing the patch.

comment:9 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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