Django

Code

Ticket #2891 (closed: wontfix)

Opened 3 years ago

Last modified 1 year ago

ADMIN_MEDIA_PREFIX set to /admin_media/ in settings template

Reported by: radek Assigned to: nobody
Milestone: Component: Core framework
Version: SVN Keywords:
Cc: cgrady@the-magi.us Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

I propose to change default settings for ADMIN_MEDIA_PREFIX in http://code.djangoproject.com/browser/django/trunk/django/conf/project_template/settings.py

instead of: ADMIN_MEDIA_PREFIX = '/media/' to use: ADMIN_MEDIA_PREFIX = '/adminmedia/'

so it will not confuse when setting MEDIA_URL to /media/ (as is suggested in several documents)

Attachments

patch (0.6 kB) - added by radek on 03/12/07 15:42:08.
patch
admin_media.patch (1.2 kB) - added by SmileyChris on 03/12/07 19:30:00.
with docs
2891.patch (1.0 kB) - added by Collin Grady <cgrady@the-magi.us> on 07/18/07 00:21:26.
patch updated against [5722]
2891.2.patch (3.6 kB) - added by SmileyChris on 08/13/07 22:33:57.
2891-backwards-incompatible.patch (3.4 kB) - added by SmileyChris on 08/13/07 22:55:41.

Change History

10/09/06 16:35:43 changed by SmileyChris

Or /admin_media/, but I like the proposal.

02/02/07 19:00:09 changed by Gary Wilson <gary.wilson@gmail.com>

  • needs_better_patch set to 1.
  • has_patch set to 1.
  • stage changed from Unreviewed to Design decision needed.
  • needs_docs set to 1.

#3417 also talks about changing ADMIN_MEDIA_PREFIX.

This would need a real patch and some documentation.

03/12/07 15:42:08 changed by radek

  • attachment patch added.

patch

03/12/07 15:45:21 changed by radek

  • summary changed from ADMIN_MEDIA_PREFIX set to /adminmedia/ in settings template to [patch] ADMIN_MEDIA_PREFIX set to /adminmedia/ in settings template.

added patch

03/12/07 19:30:00 changed by SmileyChris

  • attachment admin_media.patch added.

with docs

03/12/07 19:30:26 changed by SmileyChris

  • needs_better_patch deleted.
  • needs_docs deleted.

07/18/07 00:21:26 changed by Collin Grady <cgrady@the-magi.us>

  • attachment 2891.patch added.

patch updated against [5722]

07/18/07 00:23:11 changed by Collin Grady <cgrady@the-magi.us>

  • cc set to cgrady@the-magi.us.

I also believe this should be changed, it catches a ridiculous amount of people trying to run through things in the django IRC channel, since /media/ is so common a choice for site media

08/03/07 18:30:54 changed by SmileyChris

  • summary changed from [patch] ADMIN_MEDIA_PREFIX set to /adminmedia/ in settings template to [patch] ADMIN_MEDIA_PREFIX set to /admin_media/ in settings template.
  • stage changed from Design decision needed to Ready for checkin.

It's a non-critical change which makes things clearer. I'm going to just promote to ready for checkin.

08/05/07 00:26:43 changed by gwilson

  • needs_better_patch set to 1.
  • summary changed from [patch] ADMIN_MEDIA_PREFIX set to /admin_media/ in settings template to ADMIN_MEDIA_PREFIX set to /admin_media/ in settings template.
  • stage changed from Ready for checkin to Accepted.

There are a few other places we would probably want to make the change also, namely:

  • django/docs/fastcgi.txt
  • django/contrib/admin/templatetags/admin_modify (include_admin_script docstring)
  • django/conf/global_settings.py

08/13/07 22:33:57 changed by SmileyChris

  • attachment 2891.2.patch added.

08/13/07 22:36:51 changed by SmileyChris

  • needs_better_patch deleted.
  • stage changed from Accepted to Ready for checkin.

Ok, I've changed it in all the appropriate places now. For complete backwards compatibility, global_settings.py needs to remain as '/media/'.

The documentation now clarifies that the fall-back default is still '/media/' but the new default in a settings.py file created with django-admin.py startproject is '/admin_media/'.

08/13/07 22:55:41 changed by SmileyChris

  • attachment 2891-backwards-incompatible.patch added.

08/13/07 22:57:25 changed by SmileyChris

  • stage changed from Ready for checkin to Design decision needed.

Talking with Collin, he thinks that we should just change the default to '/admin_media/' and mention it in the BackwardsIncompatibleChanges.

Either patch is ready for checkin, but needs a decision on which way to go.

08/14/07 01:13:11 changed by mtredinnick

I would tend to side with Collin on this one, because ADMIN_MEDIA_PREFIX appears in settings.py by default, so few systems using that will fall back to the default in global_settings. Hence the impact will be minor and having global_settings and settings being the same is a win. Still want to think about it a bit more to think of hidden traps, but I suspect the second way and wearing the slight backwards-compat breakage is the more logical approach.

10/26/07 17:30:54 changed by SmileyChris

  • stage changed from Design decision needed to Ready for checkin.

Ok, I still see the only "trap" is anyone who has removed the ADMIN_MEDIA_PREFIX from their settings and is just relying on the global setting will have breakage, but this is an edge case.

Let's just do it.

12/01/07 18:53:14 changed by jacob

  • stage changed from Ready for checkin to Design decision needed.

Given that there's disagreement between a couple of core devs (me and Malcolm), this isn't actually ready for checkin at all.

12/01/07 18:58:39 changed by jacob

  • status changed from new to closed.
  • resolution set to wontfix.

OK, Malcolm and I have discussed this IRL and decided that this ticket will just break too many people's code. We should instead change the documentation to not suggest that people set MEDIA_URL to "/media/" also.

12/01/07 21:27:22 changed by SmileyChris

Bit disappointed with the wontfix, /media/ is the most obvious choice for MEDIA_URL.

I wrote a backwards compatible patch too y'know...

12/04/07 19:46:41 changed by Collin Grady <cgrady@the-magi.us>

I don't see how it would break many people's code at all - do you really think there are that many people who completely remove ADMIN_MEDIA_PREFIX from settings.py?

12/18/07 06:46:09 changed by Uninen

This is just plain silly. We're nearing 1.0 (or whatever it will be called) release which will have backwards incompatible changes, and everyone agrees that we want to have best possible APIs and defaults by then. Suggesting that MEDIA_URL should not be "/media/" and that ADMIN_MEDIA_PREFIX *should* be "/media/" is just, well, wrong.

The thing I love about Django is that it encourages to do things the right way. To me it feels like changing documentation here is the easy way out instead of fixing the real problem, whatever it is.

No offence to anyone, just wanted to share my thoughts.

04/22/08 02:49:55 changed by sander@steffann.nl

I agree with Uninen. I now have to change this manually for every site/project I create. The current name is confusing and conflicts with the most obvious situation: having MEDIA_URL = '/media/'. Existing sites already have ADMIN_MEDIA_PREFIX = '/media/' and won't be broken by applying this patch, and even if they removed ADMIN_MEDIA_PREFIX the backwards-compatibility patch can solve that.

Please reconsider this patch. The way it is currently done is _not_ the best way. Please fix this while still possible before the 1.0 release.

04/22/08 13:53:41 changed by anonymous

Sander, I encourage you to bring this up on the django-dev Google group if you wish to discuss it rather than just posting in a wontfixed ticket.


Add/Change #2891 (ADMIN_MEDIA_PREFIX set to /admin_media/ in settings template)




Change Properties
Action