Re: URL namespaces

2015-06-12 Thread Aymeric Augustin
2015-06-12 0:38 GMT+02:00 Marten Kenbeek :

> 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.
>

Some of these tests may not be up to modern standards of writing good tests.

At least that's what I remember from refactoring them when I turned the
template engine into a library.

Sometimes git blame helps find the original author's intentions and improve
the tests.

-- 
Aymeric.

-- 
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/CANE-7mXhLMCkd5WH%2BSD3Lo6HOW9w8UGUhquc7cJrn6KpVntjFw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: URL namespaces

2015-06-11 Thread Marten Kenbeek
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 
 
  
 -- 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 

Re: URL namespaces

2015-06-11 Thread 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 
>>> 
>>>  
>>> -- 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 

Re: URL namespaces

2015-06-08 Thread Aymeric Augustin
> On 8 juin 2015, at 15:24, Marten Kenbeek  wrote:
> 
> If there is a consensus that thread-local storage is the better solution

I don’t think there’s consensus when only one person brought up the idea :-)

I’m unconvinced at this time and I’ll probably argue against it if it gets more 
seriously considered.

-- 
Aymeric.




-- 
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/51E1F2FB-593E-4224-B1B3-66A8BD5A7E23%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.


Re: URL namespaces

2015-06-08 Thread Marten Kenbeek
I actually don't think this behaviour should extend to reverse(). There are 
good reasons to use request.current_app in the template, but I don't 
believe this implicit behaviour is warranted outside the templates. Not 
everyone might agree, but I also believe that get_absolute_url() should 
return a single canonical url, independent of the current app. Using the 
current app wouldn't even make sense in most cases, such as the admin or 
pretty much any app other than the model's app itself. I also plan to 
change or replace get_absolute_url somewhere in the next few months, so 
that use case of thread-local storage will soon become obsolete if it's up 
to me. 

If there is a consensus that thread-local storage is the better solution, 
I'll follow suit, but I don't plan on implementing it in this patch. It is 
easily implemented as a third-party app if it suits your use cases. 

I'd like to focus on the best way to solve the backwards compatibility 
issues. 

Op maandag 8 juni 2015 02:32:58 UTC+2 schreef Tai Lee:
>
> I still think that even though `get_absolute_url` might be disliked these 
> days for various reasons, it is very common in the wild and is a perfect 
> example of a legitimate need to be able to reverse URLs in code that 
> executes in response to a request, but that doesn't have the request passed 
> to it as an argument.
>
> So I'd still like to consider the option of adding a default `current_app` 
> hint to thread local storage (which has a life cycle of one request), and 
> for `reverse()` and therefore also `{% url %}` to use that hint as a 
> default. It could still be overridden by explicitly passing `current_app` 
> to `reverse()` (and also `{% url %}`).
>
> This could be done with a middleware class, and in fact a quick search 
> reveals there are many django snippets and blog posts out where users have 
> re-implemented a `ThreadLocalRequestMiddleware` or similar. But I would 
> prefer to see it done before middleware is applied to make it non-optional, 
> since the value will be read in `reverse()` which can and is often called 
> completely outside the request/response cycle.
>
> For any such code that executes outside the request/response cycle, I 
> think we could provide a context manager that sets the `current_app` 
> default hint on `__enter__` and restores it on `__exit__`.
>
> Django appears to use thread locals already to set the current active 
> language. It's been said that is more of a wart than a design pattern to 
> follow, but clearly it can be used safely in Django, and just like the 
> current active language, having apps that are able to resolve their own 
> URLs properly and consistently is an important feature.
>
> Here's a relevant conversation from a couple years back: 
> https://groups.google.com/forum/#!topic/django-developers/VVAZMWi76nA
>
> Cheers.
> Tai.
>
>
> On Monday, June 8, 2015 at 5:29:56 AM UTC+10, 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 

Re: URL namespaces

2015-06-07 Thread Tai Lee
I still think that even though `get_absolute_url` might be disliked these 
days for various reasons, it is very common in the wild and is a perfect 
example of a legitimate need to be able to reverse URLs in code that 
executes in response to a request, but that doesn't have the request passed 
to it as an argument.

So I'd still like to consider the option of adding a default `current_app` 
hint to thread local storage (which has a life cycle of one request), and 
for `reverse()` and therefore also `{% url %}` to use that hint as a 
default. It could still be overridden by explicitly passing `current_app` 
to `reverse()` (and also `{% url %}`).

This could be done with a middleware class, and in fact a quick search 
reveals there are many django snippets and blog posts out where users have 
re-implemented a `ThreadLocalRequestMiddleware` or similar. But I would 
prefer to see it done before middleware is applied to make it non-optional, 
since the value will be read in `reverse()` which can and is often called 
completely outside the request/response cycle.

For any such code that executes outside the request/response cycle, I think 
we could provide a context manager that sets the `current_app` default hint 
on `__enter__` and restores it on `__exit__`.

Django appears to use thread locals already to set the current active 
language. It's been said that is more of a wart than a design pattern to 
follow, but clearly it can be used safely in Django, and just like the 
current active language, having apps that are able to resolve their own 
URLs properly and consistently is an important feature.

Here's a relevant conversation from a couple years 
back: https://groups.google.com/forum/#!topic/django-developers/VVAZMWi76nA

Cheers.
Tai.


On Monday, June 8, 2015 at 5:29:56 AM UTC+10, 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. 

Re: URL namespaces

2015-06-07 Thread Marten Kenbeek
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 
>> 
>>  
>> -- 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 

Re: URL namespaces

2015-06-02 Thread 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 
> 
>  
> -- 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/648e907a-6d58-44a4-b854-a7a8cba860ea%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: URL namespaces

2015-05-30 Thread Tai Lee

>
> An app doesn't necessarily have to use its own patterns. With that said, 
> it's up to the user to make sure it works if they are to mess with the 
> app_name. Two conflicting apps can be installed with different namespaces, 
> but there will always be a default (the last one, if there is no namespace 
> with the same name). With a consistent use of `current_app` (even if it's 
> not the *current* application used in the request), you can avoid problems.
>

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.

If possible, I'd like to add `current_app` to the request by default, but 
> that has been attempted before, and backwards compatibility is a problem. 
> That would allow an app to more reliably reverse its own urls, provided 
> that their code and templates are called from within their own application.
>

I'd take this a step further. In this comment -- 
https://code.djangoproject.com/ticket/21927#comment:9 -- 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.

-- 
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/ba7b83e0-8961-45f9-89f0-e45ce25224fb%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: URL namespaces

2015-05-29 Thread Marten Kenbeek

>
> I think it's an unlikely case anyway, but out of curiosity what are the 
> workarounds?


In the case of a 2-tuple, just pass (admin.site.urls[0], 'dashboard'). In 
the case of a module, you'll have to directly import the module and pass a 
(patterns, app_name) tuple. 

If an app has templates and view code that are reversing 
> :, then changing the app name (e.g. by an undocumented 
> feature, passing a (patterns, app_name) tuple), then all those URLs will 
> break. Unless I'm missing something, an app *must always* be installed with 
> the app name that is hard coded in its templates and view code, so there is 
> no workaround for app namespace conflicts?
>
 

Does or would it work to allow two apps with the same app namespace, as 
> long as they both are installed with customised instance names, so that the 
> "default app" namespace lookup doesn't return anything, and Django must 
> fallback to the `current_app` to determine which app is being reversed?


 An app doesn't necessarily have to use its own patterns. With that said, 
it's up to the user to make sure it works if they are to mess with the 
app_name. Two conflicting apps can be installed with different namespaces, 
but there will always be a default (the last one, if there is no namespace 
with the same name). With a consistent use of `current_app` (even if it's 
not the *current* application used in the request), you can avoid problems. 

If possible, I'd like to add `current_app` to the request by default, but 
that has been attempted before, and backwards compatibility is a problem. 
That would allow an app to more reliably reverse its own urls, provided 
that their code and templates are called from within their own application. 

I'd rather see the "app name, period" (as opposed to a default app name) 
> defined as a variable in an app's URLconf as per #11642 or as an attribute 
> of the `urlpatterns` object, that Django looks at when a URLconf is 
> included with a dotted path string, instead of requiring users to import 
> and then include a `(patterns, app_name)` tuple. Basically, the app name 
> defined by the app should be used for both of:
>
 

include(r^foo/', 'foo.urls')
>
 

and
>
 

from foo.urls import some_tuple
> include(r'^foo/', some_tuple),


I also prefer the first option. That would be the "main" way of setting the 
app_name. The second option exists for things like `admin.site.urls`, which 
historically have always returned a tuple, instead of an object with an 
`urlpatterns` attribute. If objects like `admin.site` would have 
`urlpatterns` and `app_name` attributes/properties, this could be 
deprecated in favour of a single way to provide an app_name. Not sure if 
this is desirable, but it's an option. 

Not exactly related to the issue at hand, but if we are reworking URL 
> namespaces it might be worth some consideration. One reason why I always 
> avoid using URL namespaces these days is that it becomes impossible to 
> override an app URL at the project level. Project authors are stuck with 
> the exact URLs defined by the app author, all located below the same root 
> path where the URLs are included and with the same view behaviour.
>
 

Without namespaced URLs, the project author can include their own version 
> of a URL at any location (as long as it comes before the app's included 
> URLs) and change the view behaviour. For namespaced URLs, I'd like to be 
> able to do something like:
>
 

urlpatterns = patterns('',
> url(r'^my_foo_login/$', 'myapp.views.my_foo_login', name='login', 
> app='foo', namespace='foo'),
> url(r'^foo/', include('foo.urls', app='foo', namespace='foo')),
> )
>
 

Then when templates or view code in the `foo` app does reverse('foo:login') 
> they will get `/my_foo_login/` instead of `/foo/login/` and I can change 
> the behaviour of the view.


Now that's something I hadn't thought about. I don't believe there is an 
easy solution, at least not with the current API, so this will probably 
have to wait for the new API. I'll make sure to think this through, and 
find a way to handle this before I start working on the final 
implementation. 

Marten

Op vrijdag 29 mei 2015 02:52:24 UTC+2 schreef Tai Lee:
>
> Thanks for taking the time to write up this proposal. I agree on pretty 
> much all counts, but I do have a few comments:
>
> 1)
>
> If an app has templates and view code that are reversing 
> :, then changing the app name (e.g. by an undocumented 
> feature, passing a (patterns, app_name) tuple), then all those URLs will 
> break. Unless I'm missing something, an app *must always* be installed with 
> the app name that is hard coded in its templates and view code, so there is 
> no workaround for app namespace conflicts?
>
> Does or would it work to allow two apps with the sa

Re: URL namespaces

2015-05-28 Thread Tai Lee
Thanks for taking the time to write up this proposal. I agree on pretty 
much all counts, but I do have a few comments:

1)

If an app has templates and view code that are reversing 
:, then changing the app name (e.g. by an undocumented 
feature, passing a (patterns, app_name) tuple), then all those URLs will 
break. Unless I'm missing something, an app *must always* be installed with 
the app name that is hard coded in its templates and view code, so there is 
no workaround for app namespace conflicts?

Does or would it work to allow two apps with the same app namespace, as 
long as they both are installed with customised instance names, so that the 
"default app" namespace lookup doesn't return anything, and Django must 
fallback to the `current_app` to determine which app is being reversed?

2)

I'd rather see the "app name, period" (as opposed to a default app name) 
defined as a variable in an app's URLconf as per #11642 or as an attribute 
of the `urlpatterns` object, that Django looks at when a URLconf is 
included with a dotted path string, instead of requiring users to import 
and then include a `(patterns, app_name)` tuple. Basically, the app name 
defined by the app should be used for both of:

include(r^foo/', 'foo.urls')

and

from foo.urls import some_tuple
include(r'^foo/', some_tuple),

3)

Not exactly related to the issue at hand, but if we are reworking URL 
namespaces it might be worth some consideration. One reason why I always 
avoid using URL namespaces these days is that it becomes impossible to 
override an app URL at the project level. Project authors are stuck with 
the exact URLs defined by the app author, all located below the same root 
path where the URLs are included and with the same view behaviour.

Without namespaced URLs, the project author can include their own version 
of a URL at any location (as long as it comes before the app's included 
URLs) and change the view behaviour. For namespaced URLs, I'd like to be 
able to do something like:

urlpatterns = patterns('',
url(r'^my_foo_login/$', 'myapp.views.my_foo_login', name='login', 
app='foo', namespace='foo'),
url(r'^foo/', include('foo.urls', app='foo', namespace='foo')),
)

Then when templates or view code in the `foo` app does reverse('foo:login') 
they will get `/my_foo_login/` instead of `/foo/login/` and I can change 
the behaviour of the view.

Cheers.
Tai.


On Friday, May 29, 2015 at 1:21:58 AM UTC+10, Marten Kenbeek wrote:
>
> Sure, I'll go through the list of url tickets on Trac. 
>
> The key difference with #11642 is "default". What I propose is that 
> urls.py specifies the app_name, period. The only reason to change app_name 
> is conflicting app names, and there are easy workarounds that I feel 
> shouldn't be part of the API itself. The namespace would be specified in 
> include(), and default to the app_name. 
>
> Op donderdag 28 mei 2015 15:37:20 UTC+2 schreef Tim Graham:
>>
>> Point 1 sounds like https://code.djangoproject.com/ticket/11642 -- and 
>> that ticket says it may be superseded by 
>> https://code.djangoproject.com/ticket/21927. Could you review those 
>> tickets as well as the others in the "Core (URLs)" component of Trac? It 
>> would be good if you could assign yourself any tickets that your project 
>> will address.
>>
>> On Wednesday, May 27, 2015 at 5:58:00 PM UTC-4, Marten Kenbeek wrote:
>>>
>>> Hi all,
>>>
>>> URL namespaces have a few quirks that make them a bit difficult to use 
>>> and understand. I hope to create a patch that clears up these issues. 
>>>
>>> First up: the distinction between an application namespace and an 
>>> instance namespace. The docs say on the application namespace:
>>>
>>> This describes the name of the application that is being deployed. Every 
>>>> instance of a single application will have the same application namespace.
>>>
>>>
>>> And on the instance namespace: 
>>>
>>> This identifies a specific instance of an application. Instance 
>>>> namespaces should be unique across your entire project. However, an 
>>>> instance namespace can be the same as the application namespace. This is 
>>>> used to specify a default instance of an application. 
>>>
>>>
>>> The current implementation requires that both are specified in the same 
>>> place: either in the included url configuration by returning a 3-tuple, or 
>>> in the including url configuration through the arguments to include(). The 
>>> first case generally does not allow multiple deployments of the same app, 
>>> unless the included url configuration contains specific logic to return 
>>> different instance 

Re: URL namespaces

2015-05-28 Thread Tim Graham
I think it's an unlikely case anyway, but out of curiosity what are the 
workarounds? Working up a patch for the first issue sounds good to me. In 
the meantime, I'll continue refreshing myself of the rest of namespaces so 
I can reply to your other points.

On Thursday, May 28, 2015 at 11:21:58 AM UTC-4, Marten Kenbeek wrote:
>
> Sure, I'll go through the list of url tickets on Trac. 
>
> The key difference with #11642 is "default". What I propose is that 
> urls.py specifies the app_name, period. The only reason to change app_name 
> is conflicting app names, and there are easy workarounds that I feel 
> shouldn't be part of the API itself. The namespace would be specified in 
> include(), and default to the app_name. 
>
> Op donderdag 28 mei 2015 15:37:20 UTC+2 schreef Tim Graham:
>>
>> Point 1 sounds like https://code.djangoproject.com/ticket/11642 -- and 
>> that ticket says it may be superseded by 
>> https://code.djangoproject.com/ticket/21927. Could you review those 
>> tickets as well as the others in the "Core (URLs)" component of Trac? It 
>> would be good if you could assign yourself any tickets that your project 
>> will address.
>>
>> On Wednesday, May 27, 2015 at 5:58:00 PM UTC-4, Marten Kenbeek wrote:
>>>
>>> Hi all,
>>>
>>> URL namespaces have a few quirks that make them a bit difficult to use 
>>> and understand. I hope to create a patch that clears up these issues. 
>>>
>>> First up: the distinction between an application namespace and an 
>>> instance namespace. The docs say on the application namespace:
>>>
>>> This describes the name of the application that is being deployed. Every 
>>>> instance of a single application will have the same application namespace.
>>>
>>>
>>> And on the instance namespace: 
>>>
>>> This identifies a specific instance of an application. Instance 
>>>> namespaces should be unique across your entire project. However, an 
>>>> instance namespace can be the same as the application namespace. This is 
>>>> used to specify a default instance of an application. 
>>>
>>>
>>> The current implementation requires that both are specified in the same 
>>> place: either in the included url configuration by returning a 3-tuple, or 
>>> in the including url configuration through the arguments to include(). The 
>>> first case generally does not allow multiple deployments of the same app, 
>>> unless the included url configuration contains specific logic to return 
>>> different instance namespaces. The second case does not in any way enforce 
>>> that multiple deployments in fact have the same application namespace. 
>>>
>>> I'd like to get the semantics and the implementation in line, and 
>>> provide one obvious way to specify namespaces. Including a set of url 
>>> patterns under a namespace involves two parts: the including urlconf that 
>>> calls include(), and the included urlconf that is imported by include(). 
>>> The first is a specific deployment of the imported urlconf; the second is a 
>>> single app. 
>>>
>>> The obvious way as I see it would be to specify the application 
>>> namespace in the app, and specify the instance namespace as a parameter to 
>>> include(). This enforces the single application namespace for a single set 
>>> of patterns, and allows any end-user to specify the instance namespace on a 
>>> per-instance basis. To take the admin as an example:
>>>
>>> admin.site.urls would return a 2-tuple: (patterns, 'admin'), where 
>>> 'admin' is the application namespace. (An alternative to a 2-tuple could be 
>>> an object with urlpatterns and app_name attributes.)
>>> To include the admin instance, use include(admin.site.urls, 
>>> namespace='admin'). This is the instance namespace. If left out, it could 
>>> default to be the same as the app name.
>>>
>>> After a deprecation period, it would be an error to specify an instance 
>>> namespace if there is no application namespace. This is to ensure that the 
>>> app can always reverse its own urls using : if it 
>>> specifies an application namespace, and using  if it doesn't. 
>>> (Setting and app_name would technically still be possible by passing a 
>>> hardcoded (patterns, app_name) tuple, just not as an advertised feature.)
>>>
>>> The second point is about nested namespace handling and current_app. 
>>>
>>> At the moment, current_app is looking for

Re: URL namespaces

2015-05-28 Thread Marten Kenbeek
Sure, I'll go through the list of url tickets on Trac. 

The key difference with #11642 is "default". What I propose is that urls.py 
specifies the app_name, period. The only reason to change app_name is 
conflicting app names, and there are easy workarounds that I feel shouldn't 
be part of the API itself. The namespace would be specified in include(), 
and default to the app_name. 

Op donderdag 28 mei 2015 15:37:20 UTC+2 schreef Tim Graham:
>
> Point 1 sounds like https://code.djangoproject.com/ticket/11642 -- and 
> that ticket says it may be superseded by 
> https://code.djangoproject.com/ticket/21927. Could you review those 
> tickets as well as the others in the "Core (URLs)" component of Trac? It 
> would be good if you could assign yourself any tickets that your project 
> will address.
>
> On Wednesday, May 27, 2015 at 5:58:00 PM UTC-4, Marten Kenbeek wrote:
>>
>> Hi all,
>>
>> URL namespaces have a few quirks that make them a bit difficult to use 
>> and understand. I hope to create a patch that clears up these issues. 
>>
>> First up: the distinction between an application namespace and an 
>> instance namespace. The docs say on the application namespace:
>>
>> This describes the name of the application that is being deployed. Every 
>>> instance of a single application will have the same application namespace.
>>
>>
>> And on the instance namespace: 
>>
>> This identifies a specific instance of an application. Instance 
>>> namespaces should be unique across your entire project. However, an 
>>> instance namespace can be the same as the application namespace. This is 
>>> used to specify a default instance of an application. 
>>
>>
>> The current implementation requires that both are specified in the same 
>> place: either in the included url configuration by returning a 3-tuple, or 
>> in the including url configuration through the arguments to include(). The 
>> first case generally does not allow multiple deployments of the same app, 
>> unless the included url configuration contains specific logic to return 
>> different instance namespaces. The second case does not in any way enforce 
>> that multiple deployments in fact have the same application namespace. 
>>
>> I'd like to get the semantics and the implementation in line, and provide 
>> one obvious way to specify namespaces. Including a set of url patterns 
>> under a namespace involves two parts: the including urlconf that calls 
>> include(), and the included urlconf that is imported by include(). The 
>> first is a specific deployment of the imported urlconf; the second is a 
>> single app. 
>>
>> The obvious way as I see it would be to specify the application namespace 
>> in the app, and specify the instance namespace as a parameter to include(). 
>> This enforces the single application namespace for a single set of 
>> patterns, and allows any end-user to specify the instance namespace on a 
>> per-instance basis. To take the admin as an example:
>>
>> admin.site.urls would return a 2-tuple: (patterns, 'admin'), where 
>> 'admin' is the application namespace. (An alternative to a 2-tuple could be 
>> an object with urlpatterns and app_name attributes.)
>> To include the admin instance, use include(admin.site.urls, 
>> namespace='admin'). This is the instance namespace. If left out, it could 
>> default to be the same as the app name.
>>
>> After a deprecation period, it would be an error to specify an instance 
>> namespace if there is no application namespace. This is to ensure that the 
>> app can always reverse its own urls using : if it 
>> specifies an application namespace, and using  if it doesn't. 
>> (Setting and app_name would technically still be possible by passing a 
>> hardcoded (patterns, app_name) tuple, just not as an advertised feature.)
>>
>> The second point is about nested namespace handling and current_app. 
>>
>> At the moment, current_app is looking for an exact namespace match. 
>> Unlike the namespaced view path, it is not split into namespace parts using 
>> current_app.split(':'). Take the following (fictional) urlpatterns:
>>
>> blog_patterns = [
>> url(r'^comments-one/', include('comments.urls', 'comments-one', 
>> 'comments')),
>> url(r'^comments-two/', include('comments.urls', 'comments-two', 
>> 'comments')),
>> ]
>>
>> urlpatterns = [
>> url(r'^blog-one/', include(blog_patterns, 'blog-one', 'blog')),
>> url(r'^blog-two/', include(blog_patterns, 'blog-two', 'blog')),
>> ]
>>
>&

Re: URL namespaces

2015-05-28 Thread Tim Graham
Point 1 sounds like https://code.djangoproject.com/ticket/11642 -- and that 
ticket says it may be superseded by 
https://code.djangoproject.com/ticket/21927. Could you review those tickets 
as well as the others in the "Core (URLs)" component of Trac? It would be 
good if you could assign yourself any tickets that your project will 
address.

On Wednesday, May 27, 2015 at 5:58:00 PM UTC-4, Marten Kenbeek wrote:
>
> Hi all,
>
> URL namespaces have a few quirks that make them a bit difficult to use and 
> understand. I hope to create a patch that clears up these issues. 
>
> First up: the distinction between an application namespace and an instance 
> namespace. The docs say on the application namespace:
>
> This describes the name of the application that is being deployed. Every 
>> instance of a single application will have the same application namespace.
>
>
> And on the instance namespace: 
>
> This identifies a specific instance of an application. Instance namespaces 
>> should be unique across your entire project. However, an instance namespace 
>> can be the same as the application namespace. This is used to specify a 
>> default instance of an application. 
>
>
> The current implementation requires that both are specified in the same 
> place: either in the included url configuration by returning a 3-tuple, or 
> in the including url configuration through the arguments to include(). The 
> first case generally does not allow multiple deployments of the same app, 
> unless the included url configuration contains specific logic to return 
> different instance namespaces. The second case does not in any way enforce 
> that multiple deployments in fact have the same application namespace. 
>
> I'd like to get the semantics and the implementation in line, and provide 
> one obvious way to specify namespaces. Including a set of url patterns 
> under a namespace involves two parts: the including urlconf that calls 
> include(), and the included urlconf that is imported by include(). The 
> first is a specific deployment of the imported urlconf; the second is a 
> single app. 
>
> The obvious way as I see it would be to specify the application namespace 
> in the app, and specify the instance namespace as a parameter to include(). 
> This enforces the single application namespace for a single set of 
> patterns, and allows any end-user to specify the instance namespace on a 
> per-instance basis. To take the admin as an example:
>
> admin.site.urls would return a 2-tuple: (patterns, 'admin'), where 'admin' 
> is the application namespace. (An alternative to a 2-tuple could be an 
> object with urlpatterns and app_name attributes.)
> To include the admin instance, use include(admin.site.urls, 
> namespace='admin'). This is the instance namespace. If left out, it could 
> default to be the same as the app name.
>
> After a deprecation period, it would be an error to specify an instance 
> namespace if there is no application namespace. This is to ensure that the 
> app can always reverse its own urls using : if it 
> specifies an application namespace, and using  if it doesn't. 
> (Setting and app_name would technically still be possible by passing a 
> hardcoded (patterns, app_name) tuple, just not as an advertised feature.)
>
> The second point is about nested namespace handling and current_app. 
>
> At the moment, current_app is looking for an exact namespace match. Unlike 
> the namespaced view path, it is not split into namespace parts using 
> current_app.split(':'). Take the following (fictional) urlpatterns:
>
> blog_patterns = [
> url(r'^comments-one/', include('comments.urls', 'comments-one', 
> 'comments')),
> url(r'^comments-two/', include('comments.urls', 'comments-two', 
> 'comments')),
> ]
>
> urlpatterns = [
> url(r'^blog-one/', include(blog_patterns, 'blog-one', 'blog')),
> url(r'^blog-two/', include(blog_patterns, 'blog-two', 'blog')),
> ]
>
> Because of how current_app is handled, it is now impossible to reverse 
> patterns in 'blog-one:comments-one:' using current_app. To select 
> 'blog-one', the current app needs to be the string 'blog-one', but to 
> select 'comments-one', it needs to be 'comments-one'. The only solution is 
> to hardcode at least one of the instance namespaces in the namespaced view 
> path. This also means that setting request.current_app to 
> request.resolver_match.namespace, as recommended in the docs, does not work 
> if you have nested namespaces. 
>
> The ResolverMatch object also has some inconsistent behaviour for 
> app_name. resolver_match.namespace is the full namespace path, i.e. 
> 'blog-one:comments-one' (with resolver_match.namespaces a list of 
> individua

URL namespaces

2015-05-27 Thread Marten Kenbeek
Hi all,

URL namespaces have a few quirks that make them a bit difficult to use and 
understand. I hope to create a patch that clears up these issues. 

First up: the distinction between an application namespace and an instance 
namespace. The docs say on the application namespace:

This describes the name of the application that is being deployed. Every 
> instance of a single application will have the same application namespace.


And on the instance namespace: 

This identifies a specific instance of an application. Instance namespaces 
> should be unique across your entire project. However, an instance namespace 
> can be the same as the application namespace. This is used to specify a 
> default instance of an application. 


The current implementation requires that both are specified in the same 
place: either in the included url configuration by returning a 3-tuple, or 
in the including url configuration through the arguments to include(). The 
first case generally does not allow multiple deployments of the same app, 
unless the included url configuration contains specific logic to return 
different instance namespaces. The second case does not in any way enforce 
that multiple deployments in fact have the same application namespace. 

I'd like to get the semantics and the implementation in line, and provide 
one obvious way to specify namespaces. Including a set of url patterns 
under a namespace involves two parts: the including urlconf that calls 
include(), and the included urlconf that is imported by include(). The 
first is a specific deployment of the imported urlconf; the second is a 
single app. 

The obvious way as I see it would be to specify the application namespace 
in the app, and specify the instance namespace as a parameter to include(). 
This enforces the single application namespace for a single set of 
patterns, and allows any end-user to specify the instance namespace on a 
per-instance basis. To take the admin as an example:

admin.site.urls would return a 2-tuple: (patterns, 'admin'), where 'admin' 
is the application namespace. (An alternative to a 2-tuple could be an 
object with urlpatterns and app_name attributes.)
To include the admin instance, use include(admin.site.urls, 
namespace='admin'). This is the instance namespace. If left out, it could 
default to be the same as the app name.

After a deprecation period, it would be an error to specify an instance 
namespace if there is no application namespace. This is to ensure that the 
app can always reverse its own urls using : if it 
specifies an application namespace, and using  if it doesn't. 
(Setting and app_name would technically still be possible by passing a 
hardcoded (patterns, app_name) tuple, just not as an advertised feature.)

The second point is about nested namespace handling and current_app. 

At the moment, current_app is looking for an exact namespace match. Unlike 
the namespaced view path, it is not split into namespace parts using 
current_app.split(':'). Take the following (fictional) urlpatterns:

blog_patterns = [
url(r'^comments-one/', include('comments.urls', 'comments-one', 
'comments')),
url(r'^comments-two/', include('comments.urls', 'comments-two', 
'comments')),
]

urlpatterns = [
url(r'^blog-one/', include(blog_patterns, 'blog-one', 'blog')),
url(r'^blog-two/', include(blog_patterns, 'blog-two', 'blog')),
]

Because of how current_app is handled, it is now impossible to reverse 
patterns in 'blog-one:comments-one:' using current_app. To select 
'blog-one', the current app needs to be the string 'blog-one', but to 
select 'comments-one', it needs to be 'comments-one'. The only solution is 
to hardcode at least one of the instance namespaces in the namespaced view 
path. This also means that setting request.current_app to 
request.resolver_match.namespace, as recommended in the docs, does not work 
if you have nested namespaces. 

The ResolverMatch object also has some inconsistent behaviour for app_name. 
resolver_match.namespace is the full namespace path, i.e. 
'blog-one:comments-one' (with resolver_match.namespaces a list of 
individual namespaces, i.e. ['blog-one', 'comments-one']), but 
resolver_match.app_name is the outer-most app_name, in this case 
'blog-one', with no trace whatsoever of the full app_name path. 

To illustrate how I would see it end up eventually (after the deprecation 
cycle), I've created a branch at 
https://github.com/knbk/django/tree/namespaces. I feel these changes are 
fairly straightforward, but any comments are appreciated. 

Marten

-- 
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/g