Skip to content

Commit

Permalink
More helpful error when STATIC_ROOT lacks prefix
Browse files Browse the repository at this point in the history 
Thanks to @dominicrodger for reporting.

Closes #36
  • Loading branch information
@evansd
evansd committed Jul 3, 2015
1 parent b8052f0 commit c8518b6
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions whitenoise/django.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class DjangoWhiteNoise(WhiteNoise):

def __init__(self, application, settings=settings):
self.configure_from_settings(settings)
self.check_settings(settings)
super(DjangoWhiteNoise, self).__init__(application)
self.add_files(self.static_root, prefix=self.static_prefix)
if self.root:
Expand All @@ -66,9 +67,16 @@ def configure_from_settings(self, settings):
self.static_prefix = get_prefix_from_url(
getattr(settings, 'STATIC_URL', ''))
self.static_root = getattr(settings, 'STATIC_ROOT', None)
if not self.static_root or self.static_prefix == '/':
raise ImproperlyConfigured('Both STATIC_URL and STATIC_ROOT '
'settings must be configured to use DjangoWhiteNoise')

def check_settings(self, settings):
if not self.static_root:
raise ImproperlyConfigured('STATIC_ROOT '
'setting must be set to a filesystem path')
if self.static_prefix == '/':
static_url = getattr(settings, 'STATIC_URL', '').rstrip('/')
raise ImproperlyConfigured('STATIC_URL setting must include a '
'URL prefix, for example: STATIC_URL = {0!r}'.format(
static_url + '/static/'))
if self.use_finders and not self.autorefresh:
raise ImproperlyConfigured('WHITENOISE_USE_FINDERS can only be '
'enabled in development when WHITENOISE_AUTOREFRESH is '
Expand Down

4 comments on commit c8518b6

@palm86
Copy link

@palm86 palm86 commented on c8518b6 Aug 11, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the STATIC_URL prefix necessary?

@evansd
Copy link
Owner Author

@evansd evansd commented on c8518b6 Aug 12, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't absolutely necessary, but it's enforced for two reasons:

  1. Django requires a non-empty STATIC_URL and, as WhiteNoise strips the hostname from the URL, if you don't have a prefix you end up with the equivalent of an empty STATIC_URL.
  2. It's bad practice to have your static files mixed up in the same namespace as the rest of your app.

@blalger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the intent, but the error message was pretty unhelpful. Do you mean "must include a path component" rather than "must include a url prefix" ? Or better yet, show the before vs after rather than just the suggested STATIC_URL. I apparently was misconfigured, and assumed all along I had the '/static/' path in my static_url, so I was going nuts trying to figure out what was wrong with my configuration.

@evansd
Copy link
Owner Author

@evansd evansd commented on c8518b6 Aug 19, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blalger Good point, thanks: "prefix" is the internal terminology, "path component" is obviously the right phrase to use in the error message; I'll change that.

I'll think about the before vs after values in the error message, but my feeling is that if people are confused about how their own settings are configured then it's hard for an error message to convince them otherwise. And, generally speaking, the more text you put in the message the less clear people find it.

Please sign in to comment.