Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to collectstatic with non-default STATICFILES_STORAGE #149

Merged
merged 3 commits into from
Jan 27, 2017
Merged

Unable to collectstatic with non-default STATICFILES_STORAGE #149

merged 3 commits into from
Jan 27, 2017

Conversation

KostyaEsmukov
Copy link
Contributor

When STATICFILES_STORAGE is set to some complicated storage (like django.contrib.staticfiles.storage.ManifestStaticFilesStorage) and DEBUG is set to off, static templatetag might raise a ValueError complaining about non-existing file (see the attached traceback).

As a result, it is impossible for collectstatic to actually create these non-existing files, unless it is run with DEBUG=True for the first time.

This is caused by filling PLUGINS during app import.

The clear solution (as for me) is to replace PLUGINS dict with a class which lazily calls static when required. I might work on it, if you think this would be an appropriate solution.

Added a failing testcase for this.

Here is a traceback of collectstatic call (django 1.9):

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/core/management/__init__.py", line 353, in execute_from_command_line
    utility.execute()
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/core/management/__init__.py", line 327, in execute
    django.setup()
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/__init__.py", line 18, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/apps/registry.py", line 85, in populate
    app_config = AppConfig.create(entry)
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/apps/config.py", line 90, in create
    module = import_module(entry)
  File "/home/.virtualenvs/vvv/lib/python3.4/importlib/__init__.py", line 109, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 2254, in _gcd_import
  File "<frozen importlib._bootstrap>", line 2237, in _find_and_load
  File "<frozen importlib._bootstrap>", line 2226, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1200, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1129, in _exec
  File "<frozen importlib._bootstrap>", line 1471, in exec_module
  File "<frozen importlib._bootstrap>", line 321, in _call_with_frames_removed
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/leaflet/__init__.py", line 204, in <module>
    _normalize_plugins_config()
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/leaflet/__init__.py", line 191, in _normalize_plugins_config
    urls[i] = static(url)
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/contrib/staticfiles/templatetags/staticfiles.py", line 9, in static
    return staticfiles_storage.url(path)
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/contrib/staticfiles/storage.py", line 131, in url
    hashed_name = self.stored_name(clean_name)
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/contrib/staticfiles/storage.py", line 280, in stored_name
    cache_name = self.clean_name(self.hashed_name(name))
  File "/home/.virtualenvs/vvv/lib/python3.4/site-packages/django/contrib/staticfiles/storage.py", line 94, in hashed_name
    (clean_name, self))
ValueError: The file 'leaflet/draw/leaflet.draw.css' could not be found with <django.contrib.staticfiles.storage.ManifestStaticFilesStorage object at 0x7f1fc2dd80b8>.

@fanquake
Copy link
Contributor

I've also been having this issue, haven't found a good fix yet.

Copy link
Collaborator

@leplatrem leplatrem left a comment

Choose a reason for hiding this comment

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

I think it's a good practice to have laziness for this kind of things.

static("a")
self.fail("static must raise an exception")
except ValueError:
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

something like self.assertRaises instead ?

_normalize_plugins_config()
finally:
staticfiles_storage._setup()
_normalize_plugins_config()
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe add some comments to explain what's expected behaviour we should have here

@KostyaEsmukov
Copy link
Contributor Author

@leplatrem Done.

I agree that this should be lazy. Unfortunately I failed (in the March) to fix this quickly, but I don't quite remember why.

@leplatrem
Copy link
Collaborator

Thanks!

A test is failing...

E..................................

======================================================================

ERROR: test_init_with_non_default_staticfiles_storage (leaflet.tests.tests.AppLoadingTest)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/travis/build/makinacorpus/django-leaflet/leaflet/tests/tests.py", line 54, in test_init_with_non_default_staticfiles_storage

    _normalize_plugins_config()

  File "/home/travis/build/makinacorpus/django-leaflet/leaflet/__init__.py", line 192, in _normalize_plugins_config

    urls[i] = static(url)

  File "/home/travis/virtualenv/python2.7.12/lib/python2.7/site-packages/django/contrib/staticfiles/templatetags/staticfiles.py", line 37, in static

    return staticfiles_storage.url(path)

  File "/home/travis/build/makinacorpus/django-leaflet/leaflet/tests/tests.py", line 25, in url

    raise ValueError

ValueError

@KostyaEsmukov
Copy link
Contributor Author

Yes, it is intended to fail. It mimics ManifestStaticFilesStorage behaviour. Once a fix will be developed so this test don't fail, the problem with collectstatic (see my first post above) will be solved.

This PR shouldn't be merged as-is. It should be extended with a fix.

I may have some time for this in the near future, but if someone else wants to take this one - you're welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants