Opened 6 years ago
Closed 6 years ago
#29570 closed Cleanup/optimization (fixed)
Add check that MEDIA_URL is not inside STATIC_URL.
Reported by: | Alejandro Dubrovsky | Owned by: | nobody |
---|---|---|---|
Component: | Core (System checks) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Herbert Fortes | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
Setting a MEDIA_URL that is within STATIC_URL, eg
STATIC_URL = '/static/'
MEDIA_URL = os.path.join(STATIC_URL, 'upped') + '/'
leads to the development server refusing to serve media files when staticfiles is installed even if a URL route is specified since the staticfiles.handlers.StaticFilesHandler takes over.
While there is a check in staticfiles warning that MEDIA_URL should not be equals to STATIC_URL, it does not warn about the cases where STATIC_URL is a prefix of MEDIA_URL. This also does not seem to be documented anywhere that I've seen.
The suggestion to allow this seems to have been closed wontfix in https://code.djangoproject.com/ticket/15199 so I won't attempt a patch. Adding a check for this case does seem easy though, and I've attached a patch that adds a warning to this ticket. The documentation could also mention that adding an explicit url pattern for static and running python manage.py runserver --nostatic
can be used as a workaround.
Attachments (1)
Change History (7)
by , 6 years ago
Attachment: | staticwarning.patch added |
---|
comment:1 by , 6 years ago
Component: | Documentation → Core (System checks) |
---|---|
Summary: | Document that MEDIA_URL inside STATIC_URL leads to unexpected behaviour in the development server → Add check that MEDIA_URL is not inside STATIC_URL. |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
Version: | 2.0 → master |
comment:2 by , 6 years ago
Needs tests: | set |
---|
Alejandro, could you create a Pull Request on GitHub with your patch, adding at least a test case, so we can review? Thanks.
comment:3 by , 6 years ago
Done (update): https://github.com/django/django/pull/10201
As mentioned in the pull request, the check only runs when staticfiles takes over (ie in DEBUG mode when staticfiles is in the INSTALLED_APPS), since that is when the problem occurs and that's what the warning warns about.
Also, I've fixed the isort failure and most of the flake8 failure but the two remaining are 'E128 continuation line under-indented for visual indent' which seems to be due to the recommended style by Django, and the related 'E125 continuation line with same indent as next logical line'.
Could someone look at that bit of code and tell me which format would be acceptable for the project please?
ie
with self.assertWarnsMessage(UserWarning, 'The contrib.staticfiles app will not serve media if MEDIA_URL is within STATIC_URL'):
comment:4 by , 6 years ago
Cc: | added |
---|
Hi,
I ran the tests (./runtests.py) without a problem.
I have one minor suggestion. Instead of \
use ()
to break a line:
if (settings.MEDIA_URL and settings.STATIC_URL and settings.MEDIA_URL.startswith(settings.STATIC_URL)):
comment:5 by , 6 years ago
Modified to use parenthesis instead of a continuation marker to break up the line as per Herbert's suggestion.
The conclusion from #15199 was that having MEDIA_URL inside STATIC_URL (MEDIA_ROOT inside STATIC_ROOT?) is a pattern that we want to avoid, so I don't think documenting a workaround is consistent with that.
Rather, strengthening the system check seems reasonable. (The patch checks media is not inside static. Is it worth doing the opposite too?)
The MEDIA_ROOT docs already say:
With the extra check this may be enough (I've always read it that way) but if an alternative phrasing that says "not inside each other too" that is both short and precise can be found then maybe we can update that as well.