The change causes exactly... 1 test failure, `shortcuts.tests.ShortcutTests.test_render`. It's not even a functional test, it only fails because `self.assertFalse(hasattr(response.context.request, 'current_app'))` fails.The template tests don't even have any namespaced urls, so `request.current_app` is pretty much untested.
"request.current_app = None" would indeed fix any broken user code. Op donderdag 11 juni 2015 21:02:59 UTC+2 schreef Tim Graham: > > About #24127, I'd like if you could investigate if making the backwards > incompatible change breaks any tests in Django's test suite. That would > offer a starting point to think about the ramifications. Wouldn't the fix > for broken user code be to set "request.current_app = None" where > necessary? > > On Sunday, June 7, 2015 at 3:29:56 PM UTC-4, Marten Kenbeek wrote: >> >> So 2) has been fixed (#24904 and #24906), and I got a PR for 1) ( >> https://github.com/django/django/pull/4724). >> >> About #24127: I see a few options here: >> >> >> 1. A context processor or similar that sets `current_app` if it isn't >> set. By default `current_app` doesn't exist, so there's a good >> distinction >> between not set and set to `None`. This would be 100% backwards >> compatible, >> but IMO it's not the best solution in the long term. >> 2. Provide a new flag to {% url %} to reverse with current_app set to >> request.resolver_match.namespace. This feels slightly less awkward than >> the >> current method recommended by the docs. >> 3. Deprecating a non-existing current_app to force an explicit choice >> between the old and new behaviour, then add the new behaviour after the >> deprecation period. This would be coupled with option 1 for ease-of-use. >> 4. A setting? A setting! Yay, another setting... >> 5. Document the slight backwards-incompatible change? And provide an >> easy way to retain the old behaviour, of course. 1 through 4 are all far >> from ideal, and I don't think there's an easy, backwards-compatible fix >> here. >> >> Thoughts? >> >> Op dinsdag 2 juni 2015 14:32:42 UTC+2 schreef Marten Kenbeek: >>> >>> The admin proves problematic. Looking at the uses of `AdminSite.name`, >>> it's not easily replaced. There are quite a few places that use the name to >>> reverse urls, but don't have access to the request and the current >>> namespace. I think the best solution for now is to document that you should >>> pass `admin.site.urls` directly to `url()`. `include()` does a bunch of >>> stuff, but none of it is needed in the case of the admin urls. This allows >>> the new `include()` API to be as intended, but it doesn't put an >>> unnecessary burden of providing the "correct" namespace for an admin >>> instance on the end-developer. >>> >>> In time I'd like to see a method on the AdminSite that returns an actual >>> resolver object, using the new API. The default URLconf template would >>> simply look something like this: >>> >>> urlpatterns = [ >>> admin.site.get_resolver('^admin/'), # or >>> get_resolver(RegexPattern('^admin/')) to be more explicit >>> ] >>> >>> This would solve the namespace issue for the admin, but it would still >>> allow for only obvious way to set the instance namespace, and keep the >>> `include()` API clean. >>> >>> Op zaterdag 30 mei 2015 14:28:22 UTC+2 schreef Tai Lee: >>>> >>>> Right. So if I understand correctly, you can avoid problems even when >>>> installing two different apps into the same app namespace as long as you >>>> *always* supply `current_app` when reversing URLs? In which case, I don't >>>> think we need to worry about being able to change the app namespace to >>>> avoid conflicts (maybe we should even disallow it), and instead just make >>>> it easier to *always* supply `current_app`, which brings me to my next >>>> point. >>>> >>> >>> Not providing an option in the API itself is the best we can do to >>> disallow it. But yes, always setting the current_app would avoid a lot of >>> problems. https://code.djangoproject.com/ticket/24127 proposes just >>> that. >>> >>> I'd take this a step further. In this comment -- >>>> https://code.djangoproject.com/ticket/21927#comment:9 >>>> <https://www.google.com/url?q=https%3A%2F%2Fcode.djangoproject.com%2Fticket%2F21927%23comment%3A9&sa=D&sntz=1&usg=AFQjCNHV0KQtjiZ8tJPhEF_AKH_JCfPInA> >>>> >>>> -- I made a suggestion that Django set a `current_app` hint in thread >>>> local >>>> storage with a middleware class or even before middleware runs, and then >>>> have `reverse()` and `{% url %}` fallback to that default hint ONLY if no >>>> current app is explicitly provided via the current mechanisms, so it >>>> should >>>> be a relatively safe change. >>>> >>>> This should make it much more convenient and possible to reverse URLs >>>> correctly regardless of conflicting app namespaces, even in code that >>>> doesn't have access to the request object. For example, model methods that >>>> are executed in templates and do not have access to a `request` or >>>> `RequestContext` object. As far as I know, 1 thread = 1 request, so using >>>> thread local storage should be safe to provide such a hint for any code >>>> that executes in response to a request. For management commands, it would >>>> still need to be supplied explicitly. What do you think? >>>> >>>> Cheers. >>>> Tai. >>>> >>> >>> I don't think it's necessary to introduce another global state. At the >>> moment there are a few places in the admin that use the name but don't have >>> access to the request, this works fine as it is, and if need be these can >>> be reworked so that the current app is passed down to these methods too. >>> The other place where this would help is `get_absolute_url()`. This only >>> adds to the list of why I don't like that method, and I personally don't >>> find it a compelling argument for introducing a global state for the >>> current app. >>> >>> I think setting the current_app on request by default is a better >>> solution that works in most cases. The only apparent issue is backwards >>> compatibility, I'm still looking for the best way to solve this. >>> >> -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/cadf6d8d-eac8-49dc-b2e3-b9a8fe091d3c%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
