Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15270 closed (fixed)

augment staticfiles_urlpatterns to accept kwarg 'document_root'

Reported by: carbonXT Owned by: jezdez
Component: contrib.staticfiles Version: master
Severity: Keywords:
Cc: carbonXT 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 jezdez 3 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 3 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 follow-up: Changed 3 years ago by 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'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 3 years ago by jezdez

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 3 years ago by jezdez

  • Owner set to jezdez

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 3 years ago by jezdez

  • milestone changed from 1.4 to 1.3

Changed 3 years ago by jezdez

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 3 years ago by jezdez

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

comment:7 Changed 3 years ago by jezdez

  • Has patch set
  • Needs documentation set
  • Needs tests set

comment:8 Changed 3 years ago by jezdez

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

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 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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.