Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#21750 closed Bug (fixed)

STATIC_ROOT is required in 1.7

Reported by: Aymeric Augustin Owned by: nobody
Component: contrib.staticfiles Version: 1.6
Severity: Release blocker Keywords:
Cc: loic@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In Django 1.7, STATIC_ROOT is required to use the staticfiles app, while it wasn't in Django 1.6 and earlier.

For instance I'm encountering the following error when running the debug toolbar's test suite.

ImproperlyConfigured: You're using the staticfiles app without having set the STATIC_ROOT setting to a filesystem path.

If this was done on purpose, it should be documented in the release notes.

Change History (9)

comment:1 by Tim Graham, 11 years ago

Triage Stage: UnreviewedAccepted
Version: master1.6

This is due to ticket #21581 which was also backported to 1.6.x but unreleased at this time. Possibly the patch was too invasive. How does the test suite use static files?

comment:2 by Aymeric Augustin, 11 years ago

That may be triggered by the Selenium tests. You can reproduce the issue simply by checking out the Django Debug Toolbar repository, removing STATIC_ROOT from tests/settings.py, putting Django 1.7 on PYTHONPATH and running "make test".

Like runserver, LiveServerTestCase doesn't require collectstatic; files can be served from their original locations. STATIC_ROOT isn't needed until one runs collectstatic. That's why the tests used to work correctly without STATIC_ROOT.

comment:3 by loic84, 11 years ago

Cc: loic@… added

@timo suggested "perhaps move the validation of STATIC_ROOT to collectstatic" on #21581. That's an option but not a pretty one since collectstatic should work with backends that don't rely on STATIC_ROOT; we'd be leaking StaticFilesStorage implementation details on the generic management command.

It's tricky because there is no need for STATIC_ROOT until collectstatic is run, but on the other hand, with STATIC_ROOT out of sight in the default template, you hit an issue the first time you run the management command, which is the finality of the staticfiles subsystem.

I lean towards a release notes patch and adding STATIC_ROOT=os.path.join(BASE_DIR, 'static') to the default template, but I'll do as instructed.

comment:4 by Marc Tamlyn, 11 years ago

The existing patch to #21581 seems to be a major inconvenience to me, and is certainly unreleasable on 1.6.x. - we can't make projects which run (or test suites which run) suddenly start breaking. I certainly have projects where the STORAGE_BACKEND is deliberately configured different during the tests (and sometimes even for local dev) to avoid the possibility of overwriting the files on S3.

It seems reasonable that collectstatic should do the check, but should do so by asking the STORAGE_BACKEND whether it is properly configured before running the command - thus not leaking the implementation details.

comment:5 by loic84, 11 years ago

This should address the backward compatibility issues while preserving the fix for #21581: https://github.com/loic/django/compare/ticket21750.

comment:6 by loic84, 11 years ago

I'm reasonably confident that this is the best way to deal with the immediate issue, so I turned it into a PR: https://github.com/django/django/pull/2156.

On IRC, @jezdez, @mjtamlyn and myself discussed a Storage.check_configured method that would raise ImperperlyConfigured if the storage is missing some settings or dependencies. I think the idea has merit as a feature of its own but that wouldn't help to solve this problem which boils down to: "validate STATIC_ROOT when STATIC_ROOT is actually used", as opposed to "validate STATIC_ROOT when the storage is used".

comment:7 by loic84, 11 years ago

Has patch: set

comment:8 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 1e9e7351f88a59261da6e616934c8283a6e3e565:

Fixed #21750 -- Fixed regression introduced by 4befb30.

Validating STATIC_ROOT in StaticFilesStorage.init turned out to be
problematic - especially with tests - because the storage refuses to work even
if there are no actual interactions with the file system, which is backward
incompatible.

Originally the validation happened in the StaticFilesStorage.path method, but
that didn't work as expected because the call to FileSystemStorage.init
replaced the empty value by a valid path. The new approach is to move back the
check to the StaticFilesStorage.path method, but ensure that the location
attribute remains None after the call to super.

Refs #21581.

comment:9 by Tim Graham <timograham@…>, 11 years ago

In 6728f159f060e536f1543b4afecb602b6693babb:

[1.6.x] Fixed #21750 -- Fixed regression introduced by 4befb30.

Validating STATIC_ROOT in StaticFilesStorage.init turned out to be
problematic - especially with tests - because the storage refuses to work even
if there are no actual interactions with the file system, which is backward
incompatible.

Originally the validation happened in the StaticFilesStorage.path method, but
that didn't work as expected because the call to FileSystemStorage.init
replaced the empty value by a valid path. The new approach is to move back the
check to the StaticFilesStorage.path method, but ensure that the location
attribute remains None after the call to super.

Refs #21581.

Backport of 1e9e7351f8 from master

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