Re: Django ORM Handling of Reverse Relationships and Multi-Value Relationships

[Marten Kenbeek]

These queries are actually not equivalent. Consider the following code:

>>> r = Related.objects.create(field='related')
>>> r.main_set.create(field_one='1', field_two='3')

>>> r.main_set.create(field_one='2', field_two='4')

>>> Related.objects.filter(Q(main__field_two='2')|Q(main__field_one='1'))
]>
>>> Related.objects.filter(main__field_two='2').filter(main__field_one='1')


Here the first queryset matches the Related instance, because it satisfies 
Q(main__field_one='1'), so one of the OR conditions is satisfied. The 
second queryset doesn't match the Related instance because 
.filter(main__field_two='2') is not true. Further filters can only reduce 
the data returned by the queryset. 

The difference between one or two filter calls is more subtle. Consider the 
following queries:

>>> Related.objects.filter(main__field_one='1').filter(main__field_two='4')
]>
>>> Related.objects.filter(main__field_one='1', main__field_two='4')


Here the first query returns the Related instance because it has a related 
Main instance with field_one='1', and it has a (different) related Main 
instance with field_two='4'. Thus it satisfies both conditions, and the AND 
condition is fulfilled. These can be different objects because of the two 
joins, but both conditions still need to be satisfied. The second query 
does not return the Related instance, because it does not have a single 
related Main instance that has both field_one='1' and field_two='4'. In 
this case there's only a single join, so the same Main instance has to 
satisfy both conditions.

On Thursday, March 29, 2018 at 11:26:44 PM UTC+2, Andrew Standley wrote:
>
> I have recently become acquainted with some ORM behaviour for reverse 
> relationships that "makes no sense", and I'm hoping someone can explain the 
> justification for the current behaviour. 
>
> This specifically relates to `filter` behaviour referenced in 29271 
> , and 16554 
>  which seems tangentially 
> related to several issues with `exclude` (24421 
> , 14645 
> , 17315 
> ) and aggregate expressions (
> 16603 , 19415 
> )
>
> Most of the confusion about 'intended' behaviour and confirmed 'bugged' 
> behaviour seems to relate to the ORM's use of joins for reverse 
> relationships.
> I think my personal confusion boils down to two questions.
>
> 1) Is there some fundamental limitation in the ORM that prevents reducing 
> the number of joins? Several of these tickets indicate how the ORM could 
> potentially produce similar results with queries that did not use multiple 
> joins. Why is that not desirable behaviour?
>
> 2) Why is the current behaviour of `filter` for multi-value relationships 
> 'intended'? I'm hoping I am missing something obvious but it seems to me 
> that `Q` objects would support the type of behaviour suggested in the 
> spanning 
> multi-valued relationships 
> 
>  documentation 
> in a much more inituative manner. In a test case with models
>
> class Related(models.Model):
> field = models.CharField(max_length=100)
>
> class Main(models.Model):
> field_one = models.CharField(max_length=100)
> field_two = models.CharField(max_length=100)
> related = models.ForeignKey(Related, on_delete=models.CASCADE)
>
>
> both
>
> >>> Related.objects.filter(Q(main__field_two='2')|Q(main__field_one='1'))
>
> SQL:
> SELECT "test_app_related"."id", "test_app_related"."field" FROM 
> "test_app_related" INNER JOIN "test_app_main" ON ("test_app_related"."id"=
>  "test_app_main"."related_id") WHERE ("test_app_main"."field_two" = "2"
>  OR "test_app_main"."field_one" = "1")
>
> and
>
> >>> Related.objects.filter(main__field_two='2').filter(main__field_one='1'
> )
>
> SQL:
> SELECT "test_app_related"."id", "test_app_related"."field" FROM "test_app_
> related" INNER JOIN "test_app_main" ON ("test_app_related"."id"= 
> "test_app_main"."related_id") INNER JOIN "test_app_main" T3 ON ("test_app_
> related"."id" = T3."related_id") WHERE ("test_app_main"."field_two" = 
> "two" AND T3."field_one" = "one")
>
> Produce exactly the same results but the second seems to have an 
> unnecessary extra join, and directly contradicts the behaviour of filter 
> with non multi-valued fields.
>
>
>
> In short, could someone be kind enough to explain the justification for 
> all this weird behaviour with multi-value relationships?
>
>
> Cheers,
>   Andrew
>

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

Re: Make bool(AnonymousUser) evaluate to false

[Marten Kenbeek]

{% if request.user %} is a perfectly valid way to check if the user object is 
available in the template. I don't think we should change that or give a 
warning.

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/bad3dbf9-a1e9-4364-9f01-53c0899dcfc1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Review of DEP 201 - simplified routing syntax

[Marten Kenbeek]

The regex in `url()` can be translated 

 
using `ugettext_lazy()`, in which case the lazy translation happens when 
`resolve()` or `reverse()` is called. It seems the current `path()` 
implementation would force evaluation when the URLs are first loaded, so 
`resolve()` and `reverse()` would use a fixed language (whichever was 
active when they were loaded) rather than allowing lazy translations.

I'm not sure how often this feature is used, but it seems like something 
`path()` should support out of the box. 

On Friday, May 12, 2017 at 12:33:28 PM UTC+2, Aymeric Augustin wrote:
>
> Hello,
>
> I reviewed the current version of DEP 201 
> 
>  as 
> well as related 
>  
> discussions 
> .
>  
> I took notes and wrote down arguments along the way. I'm sharing them 
> below. It may be useful to add some of these arguments to the DEP.
>
> Sjoerd, Tom, I didn't want to edit your DEP directly, but if you agree 
> with the items marked *[Update DEP]* below I can prepare a PR. I will now 
> take a look at the pull requests implementing this DEP.
>
>
> *Should it live as a third-party package first?*
>
> The original motivation for this DEP was to make Django easier to use by 
> people who aren't familiar with regexes.
>
> While regexes are a powerful tool, notably for shell scripting, I find it 
> counter-productive to make them a prerequisite for building a Django 
> website. You can build a very nice and useful website with models, forms, 
> templates, and the admin, without ever needing regexes — except, until now, 
> for the URLconf!
>
> Since we aren't going to say in the official tutorial "hey install this 
> third-party package to manage your URLs", that goal can only be met by 
> building the new system into Django.
>
> Besides, I suspect many professional Django developers copy-paste regexes 
> without a deep understanding of how they work. For example, I'd be 
> surprised if everyone knew why it's wrong to match a numerical id in a URL 
> with \d+ (answer at the bottom of this email).
>
> Not only is the new system easier for beginners, but I think it'll also be 
> adopted by experienced developers to reduce the risk of mistakes in 
> URLpatterns, which are an inevitable downside of their syntax. Django can 
> solve problems like avoiding \d+ for everyone.
>
> Anecdote: I keep making hard-to-detect errors in URL regexes. The only URL 
> regexes I wrote that can't be replicated with the new system are very 
> dubious and could easily be replaced with a more explicit `if 
> some_condition(request.path): raise Http404` in the corresponding view. I 
> will be happy to convert my projects to the new system.
>
> No progress was made in this area since 2008 because URL regexes are a 
> minor annoyance. After you write them, you never see them again. I think 
> that explains why no popular alternative emerged until now.
>
> Since there's a significant amount of prior art in other projects, a 
> strong consensus on DEP 201 being a good approach, and a fairly narrow 
> scope, it seems reasonable to design the new system directly into Django.
>
>
> *What alternatives would be possible?*
>
> I have some sympathy with the arguments for a pluggable URL resolver 
> system, similar to what I did for templates in Django 1.8. However I don't 
> see this happening any time soon because there's too little motivation to 
> justify the effort. As I explained above, developers tend to live with 
> whatever the framework provides.
>
> Of course, if someone wants to write a fully pluggable URL resolver, 
> that's great! But there's no momentum besides saying that "it should be 
> done that way". Furthermore, adding the new system shouldn't make it more 
> difficult to move to a fully pluggable system. If anything, it will clean 
> up the internals and prepare further work in the area. Some changes of this 
> kind were already committed.
>
> DEP 201 is mostly independent from the problem of allowing multiple views 
> to match the same URL  — 
> that is, to resume resolving URL patterns if a view applies some logic and 
> decides it can't handle a URL. This is perhaps the biggest complaint about 
> the current design of the URL resolver. Solutions include hacking around 
> the current design or changing it fundamentally.
>
> This proposal doesn't change anything to the possibility of autogenerating 
> URL structures, like DRF's routers do.
>
> I'm aware of one realistic proposals for a more elaborate and perhaps 
> cleaner system: Marten Kenbeek's dispatcher API refactor at: 
> 

Re: Review of DEP 201 - simplified routing syntax

[Marten Kenbeek]

On Friday, May 12, 2017 at 6:43:34 PM UTC+2, Florian Apolloner wrote:
>
>
>
> On Friday, May 12, 2017 at 12:33:28 PM UTC+2, Aymeric Augustin wrote:
>>
>> Django's URL resolver matches URLs as str (Unicode strings), not bytes, 
>> after percent and UTF-8 decoding. As a consequence \d matches any 
>> character in Unicode character category Nd 
>> , for 
>> example, ١ which is 1 in Arabic (unless you also specified the ASCII flag 
>> on the regex)
>>
>
> Ha, I was thinking that you might get somewhere along the lines of this. 
> That only became an issue with python 3 right? Before regex defaulted to 
> re.ASCII.
>  
>

That's not quite right. Django has actually been using the `re.UNICODE` 
flag since at least 1.0, so you'd have the same problem on Python 2. 

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d2e6048a-3fbd-43f0-8059-fa77a1c6bc7c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: DEP pre-proposal for a simpler URLs syntax.

[Marten Kenbeek]

 

>
>- The more complex part is (I think) figuring out how to deal with 
>cases where we have a `path('/', include(other_urls))` and 
>`other_urls` also has a `path()` mentioning `something`. However this 
> might 
>just be my perfectionism and paranoia seeing edge-cases where there are 
>none, or are sufficiently "edge" cases that we can suffice with a "just 
>don't do it!".
>
>
 The current resolver doesn't really handle this either. When resolving, 
the current implementation simply passes on the outermost capture group to 
the view. When reversing you can only pass in a single value that must 
match both groups. If you can solve it, great, but I think it's sufficient 
to let the underlying resolver handle it the same way. 

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d0f9d3d9-c512-4f7e-bb54-e2bf1ce2a5eb%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Template handling of undefined variables

[Marten Kenbeek]

What about adding a filter |defined that returns True if a variable is 
defined, False otherwise? It may not solve any problems when it's left 
implicit, but at least it allows users the explicitly define the behaviour 
for non-existing template variables, and it makes the intention of the 
template code crystal clear. And of course it's fully backwards compatible. 
The drawback is perhaps the resulting verbosity of if statements:

{% if user.role|defined and roles.staff|defined and user.role == 
roles.staff %}
[sensitive information here]
{% endif %}

Marten

On Monday, February 27, 2017 at 11:36:09 AM UTC+1, Luke Plant wrote:
>
>
> On 26/02/17 00:44, Karen Tracey wrote:
>
> On Sat, Feb 25, 2017 at 2:10 PM, Tim Graham  > wrote:
>
>> I think any use of undefined template variables should raise an 
>> exception. In the long run, keeping a setting to allow some other behavior 
>> seems confusing and, considering the case of templates that might be reused 
>> in different projects with different settings, even dangerous.
>>
>
> I think I'm confused...Django templates have allowed use of undefined 
> variables and documented their use as evaluating to the empty string for as 
> long as I recall. Wouldn't a change to instead raise exceptions be a major 
> backwards-incompatibility?
>
> https://docs.djangoproject.com/en/1.7/topics/templates/#variables said 
> "If you use a variable that doesn’t exist, the template system will insert 
> the value of the TEMPLATE_STRING_IF_INVALID setting, which is set to '' 
> (the empty string) by default."
>
>
> This behaviour applies only to what happens when the variable is rendered. 
> In the context of `if` tags, undefined variables get converted to `None`. 
> This behaviour is documented - 
> https://docs.djangoproject.com/en/dev/ref/templates/api/#how-invalid-variables-are-handled
>  
> but perhaps not in as much detail as necessary.
>
> The question is about changing this, especially due to templates like this:
>
> {% if user.role == roles.staff %}
> [sensitive information here]
> {% endif %}
>
> If:
> 1) you forget to provide both user and role to template context or
> 2) 'role' or 'staff' are invalid attributes, or
> 3) 'role' returns None but 'staff' is an invalid attribute, for example,
> 4) various combinations of these
>
> then this results in the sensitive info being shown, because `None == 
> None`. The proposal is to introduce a value that doesn't not compare equal 
> to itself and avoid this kind of issue.
>
> Having thought about it more, I've realised that this really isn't going 
> to work at all.
>
> First attempt:
>
> class Undefined1(object):
> def __eq__(self, other):
> return False
> 
> If we use this, then consider the following template code:
>
> {% if user.role != roles.staff %}
> This info is private, sorry
> {% else %}
> [sensitive information here]
> {% endif %}
>
>
> The default implementation of != means we will again get the sensitive 
> data shown for some of the error situations given above (e.g. 1 and 2). 
> Second attempt:
>
> class Undefined2(object):
> def __eq__(self, other):
> return False
> def __new__(self, other):
> return False
>
> This object is neither equals nor not-equals to itself or anything else. 
> But consider this template:
>
>
> {% if user.role == roles.customer %}
> This info is private, sorry
> {% else %}
> [sensitive information here]
> {% endif %}
>
> With `Undefined2` being used for invalid attributes, we again get the 
> sensitive information being shown in the case of developer error and 
> missing attributes. Plus, we now have the situation that `{% if a != b %}` 
> is not the same as `{% if not a == b %}`, which is very confusing behaviour 
> for most people.
>
> In other words, we are getting into the territory of needing a value that 
> behaves like NULL in SQL. Even if there were no implementation issues (and 
> there will be lots, because operators like 'or' 'and' etc. would need to 
> cope with this, not to mention 3rd party code), and there were no backwards 
> compatibility issues (there are lots), this would be extremely dubious to 
> me, because ternary logic is extremely tricky.
>
> The only sensible alternative to current behaviour is to raise exceptions, 
> but huge backwards incompatibility issues I think rule this out. If I were 
> to start again, I think I would make the template system not silence any 
> errors you would normally get in Python, but just have some special syntax 
> for converting non-existent data or attributes into None. But I don't have 
> that time machine.
>
> Regards,
>
> Luke
>
>

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

Re: Considering removing support for ("iLmsu") regex groups in URLpatterns. Do you use them?

[Marten Kenbeek]

This issue is actually limited to reverse(). When resolving urls, each 
nested regex is matched against the url separately, so you can just apply 
the flags to the "local" regex pattern, and get:

a/c/
a/C/
a/b/

matching, but not:

A/c/
A/C/
A/b/

The behaviour for reverse() can be a problem. For example, consider the 
following patterns:

url(r'([A-Z]+)/', include('b'))

and in b:

url(r'([A-Z]+)/$', blah, name='blah', flags=re.I)

Since the flag can only apply to the combined pattern r'([A-Z]+)/([A-Z]+)/$', 
we can either incorrectly reject reverse('blah', args=('A', 'b')), or we 
can return an invalid url for reverse('blah', args=('a', 'b')). 

This seems to be a problem with the current implementation as well, which 
would return an invalid url for reverse('blah', args=('a', 'b')), since 
(?i) applies to the whole pattern regardless of its position. 

re.I is probably the only flag we need to care about. re.U is always used, 
use of re.L is discouraged, and re.M/re.S only apply to multi-line strings. 

On Tuesday, December 20, 2016 at 5:50:39 PM UTC+1, Shai Berger wrote:
>
> I think part of Sjoerd's point was that current implementation also means 
> that 
> including the flag in a child affects parents -- but only with regard to 
> said 
> child. So, if you have 
>
> url('a/', include("b")) 
>
> and in b: 
>
> url('b/$', blah), 
> url('c/$', bleh, flags=re.I), 
>
> then the valid urls include 
>
> a/c/ 
> A/c/ 
> a/C/ 
> A/C/ 
> a/b/ 
>
> but not 
>
> A/b/ 
>
> which is a bit odd. 
>
> On Tuesday 20 December 2016 12:21:18 Adam Johnson wrote: 
> > I think the current implementation means they affect all included 
> children. 
> > 
> > On 20 December 2016 at 07:15, Sjoerd Job Postmus  > wrote: 
> > > On Mon, Dec 19, 2016 at 08:23:09AM +, Adam Johnson wrote: 
> > > > >  I guess the "safest" option is to keep the code around and let 
> this 
> > > > > 
> > > > > feature die when the deprecation ends in Python 3.7 (and meanwhile 
> > > > > see 
> > > 
> > > if 
> > > 
> > > > > anyone notices the deprecation warning in their code and files a 
> > > > > ticket about it). The only extra work compared to removing this 
> now 
> > > > > is 
> > > 
> > > silencing 
> > > 
> > > > > the Python deprecation warnings in the Django tests in the 
> meantime. 
> > > > 
> > > > Sounds fair. Flags could always be added to *url()* as an extra 
> > > 
> > > parameter. 
> > > 
> > > How would that work with nested URL patterns? Would adding a flag also 
> > > apply that for the "parent" and/or "child" patterns? Would that also 
> > > work correctly for reverse? 
> > > 
> > > Asking because I seriously don't know. 
> > > 
> > > -- 
> > > 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-develop...@googlegroups.com . 
> > > To post to this group, send email to django-d...@googlegroups.com 
> . 
> > > Visit this group at https://groups.google.com/group/django-developers. 
>
> > > To view this discussion on the web visit https://groups.google.com/d/ 
> > > msgid/django-developers/20161220071528.GA21882%40sjoerdjob.com. 
> > > For more options, visit https://groups.google.com/d/optout. 
>

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/d567e297-2afd-405d-a139-aeb9f2375924%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Rethinking migrations

[Marten Kenbeek]

On Saturday, November 5, 2016 at 4:53:49 PM UTC+1, Patryk Zawadzki wrote:
>
> 1. Dependency resolution that turns the migration dependency graph into an 
> ordered list happens every time you try to create or execute a migration. 
> If you have several hundred migrations it becomes quite slow. I'm talking 
> multiple minutes kind of slow. As you can imagine working with multiple 
> branches or perfecting your migrations quickly becomes a tedious task.
>

Did the dependency resolution actually come up in benchmarks/profiles as a 
bottleneck? When I optimized and benchmarked the dependency graph code, it 
had no trouble ordering ~1000 randomly generated migrations with lots of 
inter-app dependencies in less than a second. I'd be surprised if this had 
any significant impact on the overall performance of migrations.

An easy way to test this is the `showmigrations` command, which will only 
generate the graph without any model state changes or model rendering 
taking place. It does some other things, but nothing that should take in 
the order of minutes.

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/90cd80d5-fc72-4611-b716-b6d1e5d80e43%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Challenge teaching Django to beginners: urls.py

[Marten Kenbeek]

I actually do have working code[1] that among other things abstracts the 
regex to a Constraint, which allows you to easily inject your own custom 
logic for resolving and reversing url fragments. This allows for a "simple" 
system at a more fundamental level, rather than just translating the 
"simple" system to a regex-based system. It has full support for routing on 
any property of the request. The code is mostly in a finished state, and 
the public API is fully backwards compatible, though there are a few small 
changes in Django's current master that'll have to be merged manually or 
rewritten. The biggest hurdle is that it still doesn't have any 
documentation.

I'm afraid I don't have the time to finish it by myself at the moment, as 
I'm juggling work, a new bachelor (computer science!) and other activities. 
I'd like to continue and finish my work when I have the time (and 
motivation), but if someone were to step in where I left off, I'd be happy 
to provide any assistance required. 

[1] https://github.com/knbk/django/tree/dispatcher_api

On Friday, September 16, 2016 at 7:49:45 AM UTC+2, Marc Tamlyn wrote:
>
> Fwiw, I spent a little time investigating this a couple of years ago. I 
> would like to see Django officially bless the idea of alternative URL 
> systems, and provide the "full" regex system and preferably at least one 
> "simple" system - even if all that system supports is integers and slugs. 
> This would allow third party authors to focus on designing their "to regex" 
> translation, rather than the details of Django's resolving.
>
> I did some investigation into swapping at a "deeper" level, including 
> allowing mixed includes from one resolver type to another. This is made 
> somewhat harder by the removal of "patterns", and was much more complex. 
> However it did give much more flexibility in allowing URL patterns which 
> route based on other attributes than the path. I dont have any working 
> code, it was very conceptual. I think we should at least consider a more 
> dramatic approach in a DEP, even if it is not the intended course.
>
> Marc
>
> On 15 Sep 2016 9:52 a.m., "Sjoerd Job Postmus"  > wrote:
>
>>
>>
>> On Thursday, September 15, 2016 at 10:38:09 AM UTC+2, Michal Petrucha 
>> wrote:
>>>
>>>
>>> As cool as this idea sounds, I really don't think the URL dispatcher 
>>> is a correct component to make database queries. FWIW, I've seen 
>>> similar magic implemented in view decorators, and the thing I remember 
>>> the most from this experience was that it made it a lot harder to 
>>> follow what was happening where. 
>>>
>>> Moreover, I can imagine this turning into a complicated mess of a 
>>> syntax to allow query customizations using a weird DSL really quickly. 
>>> After all, if we allow PK lookups, it's not that unreasonable to also 
>>> want to be able to lookup by other keys, and it all goes downhill from 
>>> here. 
>>>
>>> Cheers, 
>>>
>>> Michal 
>>>
>>
>>
>> Agreed. It all goes downhill from there, so let's at least not do 
>> database queries.
>>
>> To me, that settles it: no typecasting.
>>
>> -- 
>> 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-develop...@googlegroups.com .
>> To post to this group, send email to django-d...@googlegroups.com 
>> .
>> Visit this group at https://groups.google.com/group/django-developers.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/d9db908b-d22b-428f-908a-ecdc34b8fbfb%40googlegroups.com
>>  
>> 
>> .
>> For more options, visit https://groups.google.com/d/optout.
>>
>

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/1a27fb53-269b-491c-b9e1-922b6a7a44b1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Possible Bug in RegexURLResolver

[Marten Kenbeek]

Using a singleton means that everything is cached for the lifetime of the 
process after the resolver has been populated. The thread-local is so that 
concurrent calls to _populate() can correctly determine if it is a 
recursive call within the current thread. _populate() is called *at most* 
once per thread for each language. If one thread finishes populating before 
another thread starts, that second thread can access the thread-independent 
cache and won't have to populate the resolver again.

On Thursday, July 14, 2016 at 3:15:43 PM UTC+2, Cristiano Coelho wrote:
>
> If you are using locals it means each thread will always end up calling 
> the actual populate code? Is there any point for the RegexURLResolver class 
> to be a singleton then?
>
>
> El jueves, 14 de julio de 2016, 9:12:54 (UTC-3), Marten Kenbeek escribió:
>>
>> It's not quite as simple. Concurrency is actually not the main issue 
>> here. The issue is that self._populate() only populates the app_dict, 
>> namespace_dict 
>> and reverse_dict for the currently active language. By short-circuiting 
>> if the resolver is populating, you have the chance to short-circuit while 
>> the populating thread has a different active language. The short-circuiting 
>> thread's language won't have been populated, and that will result in the 
>> above KeyError.
>>
>> The issue that self._populating is solving is that a RegexURLResolver can 
>> recursively include itself if a namespace is used. Namespaces are loaded 
>> lazily on first access, so this would generally not result in infinite 
>> recursion. But, to include all namespaced views in self._callback_strs, 
>> you need to load them immediately. self._populating prevents infinite 
>> recursion in that specific case.
>>
>> On a side note: recursively including the same resolver is possible under 
>> some limited circumstances, but so far I've only seen it happen in the 
>> Django test suite, and it doesn't even work if you don't include at least 
>> one namespace between each recursive include. It's an edge-case scenario 
>> that can be solved better by using a repeating pattern in your regex. I 
>> don't think that it provides any value, but it does significantly 
>> complicate the code. Removing this accidental "feature" would solve the 
>> issue as well, without complicating the code any further. 
>>
>> Now, to solve the issue within the constraints of the current test suite, 
>> you only need to prevent that self._populate() is called twice on the 
>> same object within the same thread. A simple thread-local object should do 
>> the trick:
>>
>> class RegexURLResolver(LocaleRegexProvider):
>> def __init__(self, regex, urlconf_name, default_kwargs=None, 
>> app_name=None, namespace=None):
>> 
>> ...
>>
>> self._local = threading.local()
>> self._local.populating = False
>> ...
>>
>>def _populate(self):
>> if self._local.populating:
>> return
>> self._local.populating = True
>> ...
>> self._local.populating = False
>>
>>
>> This will work concurrently, because all lists (lookups, namespaces, apps) 
>> are built up in local variables, and then set in 
>> self._namespace_dict[language_code] etc. as an atomic operation. The 
>> worst that can happen is that the list is overwritten atomically with an 
>> identical list. self._callback_strs is a set, so updating it with values 
>> that are already present is not a problem either.
>>
>>
>> Marten
>>
>>
>> On Wednesday, July 13, 2016 at 12:22:36 AM UTC+2, Cristiano Coelho wrote:
>>>
>>> Keep in mind your code guys is semantically different from the one of 
>>> the first post.
>>>
>>> In the first post, the _populate method can be called more than once, 
>>> and if the time window is long enough, the two or more calls will 
>>> eventually run the # ... populate code again, but on your code, the 
>>> populate code will only be called once doesn't matter how many times you 
>>> call _populate, unless the populated variable is restarted somewhere else. 
>>> So I don't know how is this exactly used but make sure to check it
>>>
>>> El martes, 12 de julio de 2016, 14:58:12 (UTC-3), Aymeric Augustin 
>>> escribió:
>>>>
>>>> On 12 Jul 2016, at 19:46, Florian Apolloner <f.apo...@gmail.com> wrote:
>>>>
>>>>
>>>> On Tuesday, July 12, 2016 at 9:25:37 AM UTC+2, Aymeric Augustin wrote:
>>

Re: Possible Bug in RegexURLResolver

[Marten Kenbeek]

It's not quite as simple. Concurrency is actually not the main issue here. 
The issue is that self._populate() only populates the app_dict, namespace_dict 
and reverse_dict for the currently active language. By short-circuiting if 
the resolver is populating, you have the chance to short-circuit while the 
populating thread has a different active language. The short-circuiting 
thread's language won't have been populated, and that will result in the 
above KeyError.

The issue that self._populating is solving is that a RegexURLResolver can 
recursively include itself if a namespace is used. Namespaces are loaded 
lazily on first access, so this would generally not result in infinite 
recursion. But, to include all namespaced views in self._callback_strs, you 
need to load them immediately. self._populating prevents infinite recursion 
in that specific case.

On a side note: recursively including the same resolver is possible under 
some limited circumstances, but so far I've only seen it happen in the 
Django test suite, and it doesn't even work if you don't include at least 
one namespace between each recursive include. It's an edge-case scenario 
that can be solved better by using a repeating pattern in your regex. I 
don't think that it provides any value, but it does significantly 
complicate the code. Removing this accidental "feature" would solve the 
issue as well, without complicating the code any further. 

Now, to solve the issue within the constraints of the current test suite, 
you only need to prevent that self._populate() is called twice on the same 
object within the same thread. A simple thread-local object should do the 
trick:

class RegexURLResolver(LocaleRegexProvider):
def __init__(self, regex, urlconf_name, default_kwargs=None, 
app_name=None, namespace=None):

...

self._local = threading.local()
self._local.populating = False
...

   def _populate(self):
if self._local.populating:
return
self._local.populating = True
...
self._local.populating = False


This will work concurrently, because all lists (lookups, namespaces, apps) 
are built up in local variables, and then set in 
self._namespace_dict[language_code] etc. as an atomic operation. The worst 
that can happen is that the list is overwritten atomically with an 
identical list. self._callback_strs is a set, so updating it with values 
that are already present is not a problem either.


Marten


On Wednesday, July 13, 2016 at 12:22:36 AM UTC+2, Cristiano Coelho wrote:
>
> Keep in mind your code guys is semantically different from the one of the 
> first post.
>
> In the first post, the _populate method can be called more than once, and 
> if the time window is long enough, the two or more calls will eventually 
> run the # ... populate code again, but on your code, the populate code will 
> only be called once doesn't matter how many times you call _populate, 
> unless the populated variable is restarted somewhere else. So I don't know 
> how is this exactly used but make sure to check it
>
> El martes, 12 de julio de 2016, 14:58:12 (UTC-3), Aymeric Augustin 
> escribió:
>>
>> On 12 Jul 2016, at 19:46, Florian Apolloner  wrote:
>>
>>
>> On Tuesday, July 12, 2016 at 9:25:37 AM UTC+2, Aymeric Augustin wrote:
>>>
>>> Can you check the condition inside the lock? The following pattern seems 
>>> simpler to me:
>>>
>>
>> The standard pattern in such cases is to check inside and outside. 
>> Outside to avoid the lock if you already populated (the majority of 
>> requests) and inside to see if another thread populated it in the time you 
>> waited to get the lock…
>>
>>
>> Yes, actually that’s what I did the last time I implemented this pattern, 
>> in Apps.populate.
>>
>> -- 
>> 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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/dfa519d1-cee3-4ed5-b4ff-a5e902b5310e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Add support for relative imports in django.conf.urls.include()?

[Marten Kenbeek]

Hi Josh,

On Monday, February 29, 2016 at 11:34:18 PM UTC+1, Josh Smeaton wrote:
>
> Going off topic a little bit but.. have we considered deprecating string 
> based includes in favour of actually importing the urls module?
>

I've tried to remove them as a proof of concept, but that's not possible 
without some small regressions and test failures. Currently, urlpatterns 
are loaded lazily, and when they're loaded, the complete content of the 
namespace is loaded. This allows you to recursively include the same 
URLconf, but only when you include the URLconf using a namespace (otherwise 
the code attempts to load every level, and you'll run into a maximum 
recursion depth error). This is done in the test suite as well[1], hence 
the test failures.

I haven't been able to think of any practical use-cases, but you never know 
what's out there. The best I can come up with is to include an arbitrary 
number of positional arguments, but I think that's better handled by 
capturing them as one argument and splitting in the view, or even better, 
by using GET parameters.

I don't think string based imports are much of a problem in the case of 
include. The import is done immediately, and the errors are clear. 
Deprecating does have the benefit of increased consistency and less magic, 
but otherwise I don't see any practical benefits.

Then users can relatively import their nested app patterns as they will.
>
 

Even if we didn't deprecate string based imports I'm wary of increasing 
> their utility.


This just made me realize that the whole problem can already be fixed from 
the user's perspective by importing the module instead of using string 
based imports. That is possible and has been possible for a long time. In 
that light, I don't see the benefit of supporting relative string based 
imports with some confusing edge-cases. 

[1] 
https://github.com/django/django/blob/eac1423f9ebcf432dc5be95d605d124a05ab2686/tests/urlpatterns_reverse/namespace_urls.py#L57
 

> On Tuesday, 1 March 2016 00:17:39 UTC+11, Tim Graham wrote:
>>
>> For example, within myproject.urls:
>>
>>
>> urlpatterns = (
>> url(r'', include('.myapp.urls')),
>> )
>>
>>
>> .. becomes equivalent to::
>>
>> urlpatterns = (
>> url(r'', include('myproject.myapp.urls')),
>> )
>>  
>>
>> Whilst a contrived example, this can make heavy-nested apps look far, far 
>> cleaner. For example, within myproject.foo.foo_bar.foo_bar_baz.urls, one 
>> only needs to include .foo_bar_baz_qux.urls to get to the "next" level, 
>> rather than the significantly more unwieldy 
>> myproject.foo.foo_bar.foo_bar_baz.foo.bar_baz_qux.urls.
>>
>> 
>>
>> What do you think of this proposal from 
>> https://code.djangoproject.com/ticket/26288? I don't know if "nested 
>> apps" are very common and/or something that should be promoted with a 
>> change like 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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/e50521cf-5535-45dc-8d50-a10073de385b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Add support for relative imports in django.conf.urls.include()?

[Marten Kenbeek]

The current approach can lead to surprising errors if include() is called 
by a helper function, rather than directly called in your urlconf. The 
relative import would be resolved in relation to the module where the 
helper function is defined. I'm not sure how much of an actual problem that 
is in practice, but it's something to keep in mind. 

On Monday, February 29, 2016 at 2:20:23 PM UTC+1, Aymeric Augustin wrote:
>
> I’m +0 on this change, for consistency with Python’s import statement.
>
> -- 
> Aymeric.
>
> On 29 Feb 2016, at 14:17, Tim Graham  
> wrote:
>
>
> For example, within myproject.urls:
>
>
> urlpatterns = (
> url(r'', include('.myapp.urls')),
> )
>
>
> .. becomes equivalent to::
>
> urlpatterns = (
> url(r'', include('myproject.myapp.urls')),
> )
>  
>
> Whilst a contrived example, this can make heavy-nested apps look far, far 
> cleaner. For example, within myproject.foo.foo_bar.foo_bar_baz.urls, one 
> only needs to include .foo_bar_baz_qux.urls to get to the "next" level, 
> rather than the significantly more unwieldy 
> myproject.foo.foo_bar.foo_bar_baz.foo.bar_baz_qux.urls.
>
> 
>
> What do you think of this proposal from 
> https://code.djangoproject.com/ticket/26288? I don't know if "nested 
> apps" are very common and/or something that should be promoted with a 
> change like 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-develop...@googlegroups.com .
> To post to this group, send email to django-d...@googlegroups.com 
> .
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/5a92150c-4969-4432-b47b-0a02ce889312%40googlegroups.com
>  
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>
>
>

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/187d6ad9-289d-446a-9932-a2c57ef3912c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: db.models.options help with a piece of code analysis.

[Marten Kenbeek]

Hi Elton,

Django makes the Meta class of abstract models available as Foo.Meta. This 
allows you to define common Meta options on the abstract base class, and 
inherit the base Meta in your concrete child models. So the above example 
won't work as you noted, but this will:

class Foo(models.Model):
class Meta:
app_label = 'app'
abstract = True

class Bar(models.Model):
class Meta(Foo.Meta):
pass

assert 'app_label' not in Bar.Meta.__dict__
assert Bar.Meta.app_label == 'app'


On Monday, January 18, 2016 at 6:34:29 PM UTC+1, Elton Pereira de Lima 
wrote:
>
> Hello charettes!
>
> Analyzing the code further, I saw that it was impossible for the Bar Meta 
> class inherits from Foo.Meta because when this code 
>  
> is executed, the Meta class ceases to exist because of this line 
> 
> .
>

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/411c9ca0-f718-4ae5-835c-fcaa6edd4cf6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Google Summer of Code 2015 statistics

[Marten Kenbeek]

Hi Himanshu,

Last summer I was the only participant in GSoC 2015 for Django. I worked on 
a complete rewrite/refactor of the URL dispatching framework. I wasn't able 
to finish my project at the time due to personal circumstances. Right now 
the project is nearing completion, I'm aiming at the next Django release. 
It still needs a few more tests, lots of documentation etc., but the bulk 
of the work is finished. The latest updates can be found 
at https://groups.google.com/forum/#!topic/django-developers/WD_mPzvGMZI 
and https://github.com/django/django/pull/5578. 

Cheers,
Marten

On Saturday, January 9, 2016 at 3:01:14 PM UTC+1, Himanshu Mishra wrote:
>
> Hello people,
>
> I was looking around for some open source organizations in GSoC 2015 list 
> who were based on Python or Django. I found out that the Django Software 
> Foundation had participated in it too ! But there are no proposals overview 
> or code submissions over melange. Most probably it means that the projects 
> were not successful but it could be something else. I am curious about 
> knowing what happened last year, which projects were selected and why were 
> they failed? Also, what is the current status of those projects?
>
> Thank you,
> Himanshu Mishra
>

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/6d2b5023-5b1c-49e6-807c-060938288fc9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: URL dispatching framework: feedback requested

[Marten Kenbeek]

The first argument to Constraint.construct() is a URL helper object which 
allows you to set the scheme, host, path, querystring and fragment 
separately. So reversing a domain constraint is as simple as this:

class DomainConstraint(Constraint):
...
def construct(self, url_object, *args, **kwargs):
url_object.host = self.domain
return url_object, args, kwargs


On Thursday, January 7, 2016 at 2:07:03 PM UTC+1, Florian Apolloner wrote:
>
>
>
> On Monday, December 28, 2015 at 5:23:19 PM UTC+1, Marten Kenbeek wrote:
>>
>> One can for example implement a DomainConstraint or MethodConstraint to 
>> match a domain name or request method in the URL, or implement a set of 
>> constraints based on the parse library for better performance than the 
>> built-in regex-based constraints.
>>
>
> A method constraint seems simple enough, how would the domain constraint 
> work with regards to reversing -- do you have an example for that?
>
> Cheers,
> Florian
>

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b3cff701-3032-4a8a-8b41-e2123fde21ff%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

URL dispatching framework: feedback requested

[Marten Kenbeek]

A quick question: Aymeric suggested to merge django.conf.urls into django.urls 
as well. Does anyone think we should not make this move?

If no one disagrees, I will proceed with a new PR to make the move.

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5e7d285c-ac2d-459e-92cd-25587d57b9ce%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Backwards-compatibility import shims

[Marten Kenbeek]

One solution is to create a RemovedInFutureVersionWarning to allow projects 
to catch the change when running with -Wall, without committing to a 
specific release. 

On Wednesday, December 30, 2015 at 3:05:32 PM UTC+1, Tim Graham wrote:
>
> To save a link click, the question is about moving 
> django.core.urlresolvers to django.urls and whether or not to start the 
> deprecating of django.core.urlresolvers immediately.
>
> I don't have a strong opinion myself. On one hand, delaying gives projects 
> more time to update, on the other, I suspect many projects will continue 
> using django.core.urlresolvers in new code if there's no indication that 
> it's deprecated.
>
> On Wednesday, December 30, 2015 at 8:50:10 AM UTC-5, Marten Kenbeek wrote:
>>
>> As noted 
>> <https://github.com/django/django/pull/5578#discussion_r44212450>by 
>> Collin on deprecating old import paths:
>>
>> I personally wouldn't mind if we didn't actually deprecate this, and left 
>>> these shims in a bit longer :). (Makes it just a hair easier for people 
>>> upgrading.) But that's just me.
>>
>>
>> There is very little maintenance overhead to keep backwards-compatible 
>> import shims longer than the current deprecation cycle.
>>
>> Any thoughts on 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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4a0e4865-b4b4-4aaf-af91-2cdc77965839%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Backwards-compatibility import shims

[Marten Kenbeek]

As noted by 
Collin on deprecating old import paths:

I personally wouldn't mind if we didn't actually deprecate this, and left 
> these shims in a bit longer :). (Makes it just a hair easier for people 
> upgrading.) But that's just me.


There is very little maintenance overhead to keep backwards-compatible 
import shims longer than the current deprecation cycle.

Any thoughts on 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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4f0d299f-1ea5-4d79-b961-461a54c622ed%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

URL dispatching framework: feedback requested

[Marten Kenbeek]

Hi everyone,

This past week I've made some great progress in rewriting the URL 
dispatcher framework and iterating on my implementation. A big part of my 
effort to refactor my original code was to increase the performance of 
reverse() to a level similar to the legacy dispatcher, and to decouple the 
various parts of the code. I think I have now achieved both goals, so I'd 
like to get some feedback on the result. 

The current code can be found at https://github.com/django/django/pull/5578.

I will be cleaning up the code and shuffling around some of it. There's 
still a lot to be done, but the high-level design and public API are pretty 
much finished and ready for review. 

The API consists of 4 parts, most of which are extendible or replaceable: a 
Dispatcher, a set of Resolvers, Constraints and the URL configuration. 

The main entry point for users is the Dispatcher class. The dispatcher is 
responsible for resolving namespaces and reversing URLs, and handles some 
of the utility functions available to users (some more may be moved here, 
such as is_valid_path() or translate_url()). It is a thin wrapper around 
the root Resolver to allow a single entry point for both reversing and 
resolving URLs. It currently provides the following public API:

   - Dispatcher.resolve(path, request=None) -> Resolve path to a 
   ResolverMatch, or raise Resolver404. 
   - Dispatcher.resolve_namespace(viewname, current_app=None) -> Resolve 
   the namespaces in viewname, taking current_app into account. Returns 
   resolved lookup in a list.
   - Dispatcher.reverse(lookup, *args, **kwargs) -> Reverse lookup, 
   consuming *args and **kwargs. Returns a full URL path or raises 
   NoReverseMatch. 
   - Dispatcher.resolve_error_handler(view_type) -> Get error handler for 
   status code view_type from current URLConf. Fall back to default error 
   handlers. 
   - Dispatcher.ready -> (bool) Whether the dispatcher is fully 
   initialized. Used to warn about reverse() where reverse_lazy() must be 
   used. 

I'm currently looking into possible thread-safety issues with 
Dispatcher.load_namespace(). There are some parts of Django that depend on 
private API's of Dispatcher and other parts of the dispatching framework. 
To maximize extensibility, I'll look if these can use public API's where 
appropriate, or gracefully fail if a swapped-in implementation doesn't 
provide the same private API. One example is admindocs, which uses the 
Dispatcher._is_callback() function for introspection. 

If a developer wishes to completely replace the dispatcher framework, this 
would be the place to do it. This will most likely be possible by setting 
request.dispatcher to a compatible Dispatcher class. 

The BaseResolver class currently has two implementations: Resolver and 
ResolverEndpoint. The resolver's form a tree structure, where each resolver 
endpoint is a leaf node that represents a view; it's job is to resolve a path 
and request to a ResolverMatch. Users will mostly use this through 
Dispatcher.resolve(), rather than using it directly. Its public API 
consists of two functions:

   - BaseResolver.resolve(path, request=None) -> Return a ResolverMatch or 
   raise Resolver404.
   - BaseResolver.describe() -> Return a human-readable description of the 
   pattern used to match the path. This is used in error messages. 

There is a slightly more extensive API that allows a resolver to "play 
nice" with Django's resolver implementations. This allows a developer to 
replace a single layer of resolvers to implement custom logic/lookups. For 
example, you can implement a resolver that uses the first hierarchical part 
of the path as a dict lookup, rather than iterating each pattern. To make 
this possible, a resolver should accept the following arguments in its 
constructor:

   - BaseResolver.__init__(pattern, decorators=None) -> pattern is a 
   URLPattern instance (explained below). decorators is a list of 
   decorators that should be applied to each view that's a "descendant" of 
   this resolver. This list is passed down so the fully decorated view can be 
   cached. 

I'm still looking how exactly we'd allow a developer to hook in a custom 
resolver, any ideas are welcome. 

Constraints are the building blocks of the current dispatcher framework. A 
Constraint can (partially) match a path and request, and extract arguments 
from them. It can also reconstruct a partial URL from a set of arguments. 
Current implementations are a RegexPattern, LocalizedRegexPattern, LocalePrefix 
and ScriptPrefix. This is the main extension point for developers. I 
envision that over time, Django will include more constraints into core for 
common use-cases. One can for example implement a DomainConstraint or 
MethodConstraint 
to match a domain name or request method in the URL, or implement a set of 
constraints based on the parse library for better performance than the 
built-in regex-based constraints. A Constraint currently has the 

Re: A community Poll

[Marten Kenbeek]

>
> I'm quite sure I was driven to that PR because importing ContentTypes 
> was causing lots of pain with AppState not being ready. 
>

Whenever I see this error message pop up, it's not because code directly 
imports ContentTypes, but because it imports some other models.py module, 
which in turn imports another models.py module before any class definition 
is reached. ContentTypes often seems to be the first module without any 
dependencies. However, it's easy enough to trace the error back to the 
models.py file that was imported first. Especially when used in a FK, 
changing it to a string reference won't help, because you're still 
importing the model with the FK before the app registry is ready. 

For what it's worth, I prefer to import models and use them directly, for 
no particular reason other than "I like it that way". 

-- 
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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/fede7124-b504-41fe-a43f-c12c6b118354%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Add an optional parameter to values() that returns a nested dictionary for foreign keys

[Marten Kenbeek]

I think it'd be more consistent with other parts of the ORM to use **kwargs 
to specify aliases. For nested data you can use an object, say N, similar 
to Q and F objects:

Articles.objects.filter(id=1).values('body', N('author'), my_custom_title=
'title')

I'm no ORM expert, but could something like this be possible by allowing 
expressions in values() and using custom output fields?

On Wednesday, November 25, 2015 at 5:09:29 PM UTC+1, Moenad wrote:
>
> Well, switch the field name aliasing to a dictionary without hijacking 
> **kwargs ?
>
> I prefer the following:
>
> Articles.objects.get(id=1).values(’title’, ’author’, ‘body', 
> alias={"title": "my_custom_title"}, nested=True)
>
>
> On Wednesday, November 25, 2015 at 5:43:51 PM UTC+2, Tim Graham wrote:
>>
>> There's an accepted ticket for adding aliasing to values(): 
>> https://code.djangoproject.com/ticket/16735
>>
>> The current patch there hijacks values() **kwargs for the mapping of 
>> renamed fields which would prevent adding other kwargs like "nested" 
>> without disallowing those values as aliases. I guess we may want to rethink 
>> that approach.
>>
>> On Wednesday, November 25, 2015 at 10:20:37 AM UTC-5, Bobby Mozumder 
>> wrote:
>>>
>>> I could also use a couple of enhancement to this:
>>>
>>> 1) Allow renaming of keys, instead of using the database column names.  
>>> 2) Allow callbacks functions (or lambdas) to convert output values to 
>>> another format if needed.
>>>
>>> With this, I could send the queries results right to JSON outputs.
>>>
>>> -bobby
>>>
>>> On Nov 25, 2015, at 9:05 AM, Moenad  wrote:
>>>
>>> Currently, after calling values() and the query executes, the output is 
>>> a single level dictionary, including foreign keys. I propose adding an 
>>> extra parameter for values, or at least values_list, where if it's set to 
>>> true, a nested dictionary will be returned when there's a foreign key.
>>>
>>> Example:
>>>
>>> Person model with the following fields: first_name, last_name and 
>>> hometown (foreign key)
>>> Hometown model with the following fields: name
>>>
>>> A single record from Person.objects.values() will looks like this
>>>
>>> {"id": 1
>>>  "first_name": "first name",
>>>  "last_name": "last name",
>>>  "hometown__id": 1,
>>>  "hometown__name": "town name",
>>> }
>>>
>>>
>>> I propose adding a nested optional parameter to values, where a single 
>>> record from Person.objects.values(nested=True) will look like
>>>
>>> {"id": 1
>>>  "first_name": "first name",
>>>  "last_name": "last name",
>>>  "hometown": {
>>>   "id": 1,
>>>   "name": "town name"
>>>   }
>>> }
>>>
>>>
>>> This feature is needed given that most APIs these days are nested, while 
>>> it's simple to implement, I think it's much better to have it a built-in 
>>> django feature.
>>>
>>>
>>> -- 
>>> 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-develop...@googlegroups.com.
>>> To post to this group, send email to django-d...@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/8e5cbc9a-0317-40d3-8038-5b4300738b90%40googlegroups.com
>>>  
>>> 
>>> .
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>>
>>>

-- 
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/32061525-207b-4dd7-99d9-45e2fa19ad9d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: __ne, #5763

[Marten Kenbeek]

The 'con' side argument is that it would create in inconsistency in the 
API, since we don't have any other negated lookups either. If we can get 
the same behaviour by fixing the current API, Django should not introduce 
an unnecessary consistency. ("Closed as wontfix by core dev" is not an 
argument against the ticket, it is a simple rule that you must discuss such 
a ticket on the mailing list before reopening it.)

Anyway, with the new lookup API, it has become trivial for any project to 
implement the __ne lookup. It is the first example in the how-to 
guide: 
https://docs.djangoproject.com/en/dev/howto/custom-lookups/#a-simple-lookup-example.

On Saturday, November 21, 2015 at 11:39:18 AM UTC+1, Carsten Fuchs wrote:
>
> Am Samstag, 21. November 2015 02:27:42 UTC+1 schrieb Aaron C. de Bruyn:
>>
>> With all due respect, looking through the ticket and reading responses 
>> shows me that the 'pro' side has good use cases for __ne, and the 'con' 
>> side basically says "use exclude" or "a core committer close the ticket, 
>> stop opening it" with no good reasoning or rationale behind it.
>>
>
> Dito.
>
> Best regards,
> Carsten
>
>

-- 
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/8d31a6bf-f435-4579-936b-3206c2c7c2e7%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Automatically get default value for "current_app", for easier namespace support.

[Marten Kenbeek]

Hi George,

The behaviour for {% url %} has since been changed when used with 
a RequestContext. It will now default to the current namespace when 
one is available. This change will be available in 1.9. 

This change doesn't cover the thread-local storage for calls to reverse(), 
or for {% url %} without a RequestContext. That is still marked as 
wont-fix. 

Marten

On Monday, October 26, 2015 at 5:44:31 AM UTC+1, George Ma wrote:
>
> That's exactly what I'm thinking about. Unfortunately, it's not brought to 
> the attention among Django developers so far.
>
> On Tuesday, 20 May 2014 02:01:55 UTC-6, Tai Lee wrote:
>>
>> Right now it seems that for a generic app to support the possibility of 
>> being installed in a URLconf with a namespace, the app author will need to 
>> take care to explicitly define a `current_app` for every call to 
>> `reverse()`, `TemplateResponse`, `RequestContext`, `Context` and `{% url 
>> %}`.
>>
>> Django already adds a `ResolverMatch` object to every request. Shouldn't 
>> Django just use this to get a default value for "current_app", whenever 
>> users don't explicitly define one?
>>
>> This should make it almost a non-issue to define the current app for 
>> every case except explicit calls to `reverse()`, `Context` and templates 
>> that are rendered without a `RequestContext` object (as none of these have 
>> access to the request).
>>
>> We could even get a sensible default in those cases as well by storing 
>> the current app in thread local storage, and using that as a default in 
>> `reverse()`.
>>
>> I've made a ticket for this, but it was closed as wontfix because thread 
>> local storage is global state and Django is at war with global state.
>>
>> https://code.djangoproject.com/ticket/22203
>>
>> As suggested, I'm bringing this to django-developers to ask for any 
>> alternative suggestions that don't involve global state, and also to try 
>> and make my case for the ticket as originally described.
>>
>> Django already uses `threading.local()` in a number of places such as 
>> `urlresolvers._prefixes`, `urlresolvers._urlconfs`, `CacheHandler._caches`, 
>> `ConnectionHandler._connections`, `trans_real._active`, `timezone._active`.
>>
>> The most notably similar use case is probably for timezone support, which 
>> allows users to call `activate()` to tell Django what timezone they are in, 
>> and then other parts of the code call `get_current_timezone()` to get the 
>> value stored in thread local storage.
>>
>> I think it would be along the same lines to have the ability to set a 
>> current app and have other parts of the code get the current app, without 
>> having to pass an object around as an argument every step of the way, which 
>> is practically impossible.
>>
>> For example, models with a `get_absolute_url` method (or perhaps multiple 
>> `get_foo_url` methods) that are rendered in templates. These functions 
>> can't take arguments when rendered as context variables in a template, and 
>> have no way to obtain the current namespace from the `request` object.
>>
>> This would make it super easy for users (via middleware) or Django itself 
>> to inspect the `request.resolver_match` object and set the current app 
>> early in the request/response cycle, and then `reverse()` and `{% url %}` 
>> would just work without generic app authors having to explicitly build in 
>> support for multiple instances of their app being deployed within a single 
>> project.
>>
>> Does anyone else think this would be a good idea, or does anyone have an 
>> alternative suggestion?
>>
>> 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/c5219869-cdcd-41cd-bd01-e1c16ebc37de%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Why is from_db_value not called on ModelClass.objects.create()?

[Marten Kenbeek]

I've had to explain this behaviour a few times - most recently this SO 
question 

 - 
so 
I think it's worth it to explicitly document this behaviour. I agree with 
Simon, though - I don't 
think the behaviour itself should change. 

You can always call refresh_from_db() 

 
to reload the values from the database. This method
was added in 1.8. Just a quick idea: adding a flag to Model.save() to call 
refresh_from_db() 
after the model is saved might make it more accessible, and it makes it 
clearer that the values 
on the model aren't changed if you don't explicitly reload the model. 

Marten

On Monday, September 28, 2015 at 10:36:09 PM UTC+2, charettes wrote:
>
> Hi Gordon,
>
> That's what I would expect the cash attribute to be since it was not 
> retrieved
> from the database but explicit set during initialization. To me the 
> `from_db_value`
> method name makes it clear it is only called in the former case. If you 
> want
> the object you created to have a certain value pass them as desired:
>
> CashModel.objects.create(cash=Cash('12.50'))
>
> or call clean() on your model instance to make sure CashField.to_python() 
> is called.
>
> Note that this behavior can also be exhibited by assigning a value to an 
> already
> existing instance.
>
> instance = CashModel.objects.get()
> instance.cash = '12.50'
> assert instance.cash == '12.50'
>
> I think it adheres to the principle of least surprise since it makes Django
> models behave just like normal Python classes in this regard.
>
> e.g.
>
> class Foo:
> def __init__(self, bar):
> self.bar = bar
>
> assert Foo('value').bar == 'value'
>
> If you really want to make sure assignment for a specific field
> sets an altered value on an instance the you'll have attach a
> descriptor on the model class and make sure to call
> CashField.to_python on __set__(). Django does this for
>
> Simon
>
> Le lundi 28 septembre 2015 15:10:54 UTC-4, Gordon a écrit :
>>
>> I originally asked this on IRC.  It was suggested to raise the question 
>> in the mailing list. I've copied the log below with irrelevant lines 
>> removed (dpastes provided after the quoted irc)
>>
>>  Why is from_db_value not called on Cls.objects.create()?  For 
>>> example, in the CashModel test, why isn't it tested that cash is an 
>>> instance of Cash after create? 
>>
>>  
>>> https://github.com/django/django/blob/master/tests/from_db_value/tests.py
>>
>>  Something like http://dpaste.com/1R69S5F  I seem to be getting the 
>>> wrong value for a custom primary key in the admin because of this
>>
>>  *only on the create redirect
>>
>>  Or is this expected to be handled by the field?
>>
>>  gp: this seems similar to #24028
>>
>>  timograhm: would that test 'test_create()' in the dpaste fit in with 
>>> the from_db_value?  Or should the create test be somewhere else?
>>
>>  seems okay there, does it pass or are you proposing a 
>>> behavior change?
>>
>>  When I was trying to figure out why my values were incorrect I tried 
>>> that test and it failed.  But I would need to verify in a clean project 
>>> before sending a pr
>>
>>  but I think the actual fix is outside of my knowledge
>>
>>  This ugly hack seems to fix it on my field if that means anything to 
>>> you http://dpaste.com/3J0C5DD
>>
>>  gp: it seems like it could be a useful behavior. I guess it 
>>> has probably been discussed before but I'm not aware of the discussion. 
>>> Maybe it would be worth raising on the mailing list and at least 
>>> documenting the reasons for the current behavior if it can't be changed due 
>>> to backwards compatibility.
>>
>>
>>  The first dpaste (1R69S5F) is a copy of the from_db_value/tests.py with 
>> the following modifications:
>>
>> class FromDBValueTest(TestCase):
>> def setUp(self):
>> self.obj = CashModel.objects.create(cash='12.50')
>>
>> def test_create(self):
>> self.assertIsInstance(self.obj.cash, Cash)
>>
>>
>> The second dpaste (3J0C5DD) is the following:
>>
>> def contribute_to_class(self, cls, name):
>> super(IntegerIdentifierBase, self).contribute_to_class(cls, name)
>> cls_save = cls.save
>> def save_wrapper(obj, *args, **kwargs):
>> cls_save(obj, *args, **kwargs)
>> value = getattr(obj, self.attname)
>> value = self.to_python(value)
>> setattr(obj, self.attname, value)
>> cls.save = save_wrapper
>>
>>
>>
>>

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

Re: Ability to migrate other applications

[Marten Kenbeek]

>
> The order of migrations isn't something which is local to this feature, it 
> is something which isn't fixed (probably by design, correct me if I'm 
> wrong) and if it is not "the right way" to do it, maybe it should be part 
> of another issue.


The order of migrations within a single app is fixed. That means the order 
of the operations on a single model or table is also fixed. 
This proposition introduces a variable order to migrations that alter the 
same model, so yeah, this issue is local to this feature. 

Once again, correct me if I'm wrong but the only way to have different 
> final rendered models would be the original third-party app 
> defining/altering a field which has already been defined/altered by your 
> app, resulting, in regards of the current proposal, in clashing fields.
> Creating a field which clashes with another app's field is something bad, 
> there are nine chances out of ten that fixed migration order will not solve 
> your overall problem: you created a field named the same as another field, 
> except if you have a psychic connection with the original library's author 
> you probably have a different usage for the field (however slight the 
> difference may be) and, if you really need to upgrade that third party 
> library, you have a whole other mess to fix than conflicting migrations so 
> I am really wondering if this is worth the hassle.
> If you create two models with the same meta db_table, django doesn't come 
> to your rescue and propose a solution, it is something you shouldn't have 
> done and now you have to deal with it. I think this "edge case" is in the 
> same area.


Since we're talking about unsupported, undocumented ways to change models 
anyway (like contribute_to_class), I think altering an existing 
field is a very legitimate reason to create a new migration. For instance, 
many people want to increase the length of the username field on the 
default User model, and the upcoming 1.9 release will also alter the 
username field again, resulting in the scenario I described above. So 
unless 
we explicitly warn against this in the documentation, we should find a 
suitable solution for this. 

Marten

Op woensdag 9 september 2015 12:02:57 UTC+2 schreef Emmanuelle Delescolle:
>
> if we change the code in Django that does order resolution even slightly 
>> it could result in different operation orders or even different "final" 
>> rendered models.
>
>
> The order of migrations isn't something which is local to this feature, it 
> is something which isn't fixed (probably by design, correct me if I'm 
> wrong) and if it is not "the right way" to do it, maybe it should be part 
> of another issue.
>
> Once again, correct me if I'm wrong but the only way to have different 
> final rendered models would be the original third-party app 
> defining/altering a field which has already been defined/altered by your 
> app, resulting, in regards of the current proposal, in clashing fields.
> Creating a field which clashes with another app's field is something bad, 
> there are nine chances out of ten that fixed migration order will not solve 
> your overall problem: you created a field named the same as another field, 
> except if you have a psychic connection with the original library's author 
> you probably have a different usage for the field (however slight the 
> difference may be) and, if you really need to upgrade that third party 
> library, you have a whole other mess to fix than conflicting migrations so 
> I am really wondering if this is worth the hassle.
> If you create two models with the same meta db_table, django doesn't come 
> to your rescue and propose a solution, it is something you shouldn't have 
> done and now you have to deal with it. I think this "edge case" is in the 
> same area.
>
> would prefer some better- 
>> looking solution -- e.g. "project migrations" (there are other reasons to 
>> think of them, like, special migrations to change swappable models); but 
>> unless some such idea gets some backing, I'd be only -0 on this. 
>>
>
> A "project" is just a module, so if you create a "migrations" directory, 
> add an __init__.py inside it and add it to your INSTALLED_APPS, you can 
> start placing migrations inside it. That's actually where I put those 
> "migrations for other apps" at the moment.
> And BTW, changing a swappable model is another "legitimate" example of a 
> migration being created in a third party app (django.contrib.admin) while 
> strictly using "recommended" features of Django.
>
> Cheers
>
>

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

Re: URL Dispatcher

[Marten Kenbeek]

Hi James,

Thanks for keeping this alive while I had some personal issues to sort out. 
I'll be working on this again, though I can use all the help I can get. 

As for the work that still needs to be done:

   - 3.1 is done, mostly as a proof-of-concept that can be found 
   at https://github.com/knbk/django/tree/url_dispatcher
   - 3.2 is done. I've worked on this in separate PR's, which have all been 
   merged by now. 
   - 3.3: 
  - Legacy functionality has been implemented, namely through the 
  RegexPattern, LocalizedRegexPattern and LocalePrefix constraints.
  - I've kept a lot of functional tests, and I believe the unit tests 
  have decent coverage. It would be good to have more tests, but I don't 
  think it's a necessity. 
  - Yes, pretty much all the documentation must still be written. 
  - These could be implemented pretty straight-forward in separate 
  patches after the main project has been merged. 
   
The resolver is not quite as fast as I'd hoped, especially `reverse()`. 
There are some trade-offs we can make, but we'll have to decide which are 
worth it. 
I think that is the main problem we need to tackle before we push for a 
merge. 

Marten

Op dinsdag 25 augustus 2015 20:57:14 UTC+2 schreef Tim Graham:
>
> Thanks for taking this on, James. I do think it would be best to defer 
> this to Django 1.10 rather than try to rush it into Django 1.9 at this 
> point. Also, there are several URL related deprecations scheduled to 
> complete in 1.10 which should simplify things (I've started on those 
> removals here: https://github.com/django/django/pull/5143). Absent 
> another interested person, I should be able to mentor you on this during 
> the Django 1.10 release cycle (which can start once we cut the stable/1.9.x 
> branch after 1.9 alpha around September 21). In the meantime, if you want 
> take a look for some simpler tickets to tackle to get a feel for our 
> contribution process, I think that would be helpful.
>
> On Tuesday, August 25, 2015 at 2:25:07 PM UTC-4, James Addison wrote:
>>
>> In the interests of keeping the excellent work done by Marten Kenbeek 
>> (knbk on IRC/Github) on the GSOC 2015 URL Dispatcher project [1] current 
>> and moving along, I've created a new branch [2] and rebased it against 
>> current master. (I am doing this because it seems like Marten is no longer 
>> able to continue working on the effort. *If this should change or is 
>> incorrect, Marten, please let me/us know!*)
>>
>> To try and find how far along the project is, I've quickly compared the 
>> original GSOC outline/timeline [1] against my updated branch [2] to see 
>> what is done/remaining:
>>
>>- section 3.1 (Design phase) I believe was done, but am unable to 
>>find anything other than a few posts on the django-developer mailing list 
>> - *is 
>>there a source for this?*
>>- section 3.2 (Namespace overhaul) hasn't been addressed that I can 
>>tell - *I'd recommend dropping this as it isn't core to the effort*
>>- Items completed in section 3.3 (Dispatcher API): 3.3.[1-4]
>>- *Items remaining* in section 3.3: (Dispatcher API): 3.3.[5-9] - 
>>some highlights:
>>   - re-implementing some legacy functionality for backward 
>>   compatibility (ie. RegexURLPattern)
>>   - additional tests
>>   - lots of documentation!
>>   - additional resolvers (ie. scheme, sub-domain, etc)
>>
>>
>>
>> Please keep in mind that I'm mostly unfamiliar with the internals of 
>> Django core elements, so I may have easily overlooked/mistook relevant 
>> information. I'd appreciate any other review/input as well. Here is the 
>> branch comparison [3] on Github.
>>
>> Based on the Django 1.9 release schedule, I have doubts that this will be 
>> merged in for feature freeze, which is unfortunate. I would like to think 
>> that I am up to the task, but the URL functionality in Django is about as 
>> 'core' as it gets and I would not want to bite off more than I can chew 
>> without backup. Any thoughts on the continued viability of this effort? Is 
>> this something that someone inexperienced with Django internals should take 
>> on? If so, I'd definitely need coaching!
>>
>> Lastly, I want to mention that I do have a new project in which I had 
>> been using Marten's branch (and am now using my own branch [2]), and it has 
>> been functionally solid for the scheme, (sub) domain and url resolving and 
>> reversing use cases. See the the urls.py [4] from that project to whet 
>> your appetites as to the possibilities of all this. :)
>>
>> Previous discussions on the django-developers mailing list [

Re: Ability to migrate other applications

[Marten Kenbeek]

Say A.0002 and B.0001 are different leaf nodes for A.0001, and both alter 
the same field. 
Now, on a clean database, the order is pretty stable. If that order is 
A.0001, A.0002, B.0001,
you'll always end up with the field from B.0001. This is also what the 
autodetector sees. 

If you first create and apply the migrations A.0001 and B.0001, and then an 
update to A adds
the new migration A.0002, the database will end up with the field from 
A.0002. The autodetector 
will still see the field from B.0001, and won't  create a new migration, so 
if the final model has 
the field from B.0001, you have a mismatching database table. 

I think the fact that the order depends on which migrations have been 
applied is enough to require 
a merge migration. 

Marten

Op dinsdag 8 september 2015 21:33:10 UTC+2 schreef Andrew Godwin:
>
> I still feel like merge migrations are necessary, if not only because 
> they're a way of reducing the potential collision area of migration 
> ordering from different branches, but it might be that the solver is 
> predictable enough that it's not a problem. Two separate branches in 
> migrations still don't have an order relative to each other, so if we 
> change the code in Django that does order resolution even slightly it could 
> result in different operation orders or even different "final" rendered 
> models.
>
> Andrew
>
> On Mon, Sep 7, 2015 at 5:16 PM, Shai Berger  > wrote:
>
>> Ok, two things:
>>
>> 1) Markus' idea of having more than one folder for migration modules seems
>> reasonable enough. I disagree with his comment about the placement of 
>> merge
>> migrations --
>>
>> > Django needs to know where to
>> > place the merge-migration. I'd go with the first item in the list
>>
>> I'd require an explicit selection by the user; I'd still want to make 
>> sure the
>> selected path is one of those specified for migration modules, so a 
>> dialog for
>> selection may be more appropriate than a command-line parameter.
>>
>> BUT
>>
>> 2) Emma's experiment, essentially, proves that the migration system can 
>> live,
>> migrate, and generate new migrations with two leaf-migrations present. 
>> Which
>> begs the question -- are merge migrations really necessary at all? I know 
>> why
>> they were necessary for South, where migrations in an app were ordered
>> linearly, but we have grown past that.
>>
>> If, as I now suspect, we actually don't need them, then the whole idea 
>> sounds
>> much more reasonable. I still feel funny about a migration which belongs 
>> to
>> one app and works on the models of another, and would prefer some better-
>> looking solution -- e.g. "project migrations" (there are other reasons to
>> think of them, like, special migrations to change swappable models); but
>> unless some such idea gets some backing, I'd be only -0 on this.
>>
>> Shai.
>>
>
>

-- 
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/f621c8bd-9c87-4c79-a99c-caa13365368f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Ability to migrate other applications

[Marten Kenbeek]

Hi Emmanuelle,

I think the main problem is the dependencies between migrations. The order 
in which migrations 
are run is not fixed. It depends on which migrations have already run, and 
we can't guarantee the 
same order for each deployment. If each chain of migrations that could act 
on a single model has 
a fixed order, it's not much of a problem. Once each app can act on each 
model, it becomes a big, 
and possibly unsolvable problem. I think the scenario you present as an 
edge-case, where two 
migrations from different apps affect the same field, is the exact case we 
have to watch for, which can 
cause significant problems in how we handle migrations. 

If we are to "solve" this, I think Markus's idea it the best solution. If 
you can make all the migrations in
a single app affect the same app (which doesn't have to be the app that 
contains them), you can make 
consistent rules on the order in which all migrations that affect the 
target app are run. Once you have 
that, it's not that different from the current situation: each migrated app 
has a single chain of migrations 
that have to be applied, with several inter-app dependencies that are 
resolved at runtime. 

The inter-app dependencies might be difficult, but at least they are 
solvable. I have reasons to believe that if 
you allow each migration file to target a different app, it becomes an 
unsolvable dependency problem. If you'd 
like I can do some more research to come to a definite conclusion. 

If anything, I think Tim's solution is still the best solution; if you have 
to manually copy the migrations to an app 
within your project, it is very clear that you need to do something when 
the third-party app publishes some 
new migrations that could conflict with the order in which the migrations 
would currently be applied. Once you 
have that in mind, it's not that much trouble to copy and merge any new 
migraitons, and I think it will save a lot of
headaches if we only allow inter-app migrations through the 
MIGRATION_MODULES settting. 

Marten

Op vrijdag 4 september 2015 08:28:48 UTC+2 schreef Emmanuelle Delescolle:
>
>
>
> On Friday, 4 September 2015 00:29:28 UTC+2, Shai Berger wrote:
>>
>> What Markus (and myself) assumed is that when you add the "migrated_app" 
>> attribute, you make the migration belong to the migrated app. What you 
>> did is 
>> different: The migration still belongs to the app it is defined in, it is 
>> only 
>> the operations in it that "belong" to the migrated app. 
>>
>
> People can actually do that today out of the box by adding those 2 lines 
> to the generated migration when moving it:
>
>> def __init__(self, name, app_label):
>> super(Migration, self).__init__(name, 'other_third_party_app')
>
> But that would break everything (It would counter-intuitively work on that 
> migration but would break right after that), It wouldn't even generate a 
> merge migration. Because the models the migration is working on would not 
> be loaded.
>
>
>> This is evidenced both by the printing ("Applying unruly.0001_initial" 
>> etc) 
>> and by the handling of leaf migrations: 
>>
>  
> No need to show me evidence, that's exactly what I am trying to do (but 
> thanks for pointing out it does work as intended)
>
> What this all means is, your patch *subverts* the dependency checks, 
>> allowing 
>> more than one leaf migration on an app (and forcing a single leaf 
>> migration 
>> where it shouldn't -- there should be no requirement for dependencies 
>> among 
>> the "unruly" migrations). This makes things even more fragile than you 
>> implied: I think after you move your migration from the original app 
>> (say, 
>> "pizzas") to your "unruly",
>
>
> You can call it perverting if you like what, it does is just say "hey this 
> migration belongs to unruly" because, that's true, this migration should 
> belong to the app that created it. But it's working on the models from 
> pizzas (because that's also what it's doing) that way, the migration 
> framework knows not to load the models from unruly but from pizzas instead.
>  
>
>> if you "makemigrations pizzas" again it will be 
>> generated again. Which means, if you have two "unruly" apps, the 
>> interactions 
>> between them become, let's say, interesting. 
>>
>  
> Interesting indeed, since screenshots work better than words or code 
> samples, please take a look at the attached screenshot.
> The reason it doesn't create a new migration is the same reason why it 
> created a migration for an app to which you didn't change a line of code in 
> the first place: the state of the model is the same state as the one 
> computed by the migration system and whether it's the maintainer of pizzas 
> (who doesn't have the unruly app in their project) or the owner of the 
> current project (who's current model's state correspond to the one 
> generated by the unruly migration) who runs that command, the model state 
> is still the same as the state generated by the 

Re: URL dispatcher API

[Marten Kenbeek]

Hi James,

Thanks for taking a look at this, I really appreciate it. 

*Major bug:* the `request` object needs to be passed into `resolve()` here: 
> https://github.com/knbk/django/blob/4a9d2a05bb76c9ad996921b9efadd8dad877540e/django/core/handlers/base.py#L134
>  
> - otherwise host and scheme constraints cannot work. I believe there are 
> other instances of `resolve()` not getting `request`, but mostly in tests. 
> Is there a way for `request` to be a required parameter instead of 
> defaulting to `None`?
>

Damn... I'll fix this asap. I haven't implemented any request-based 
constraints yet, which allowed this to slip through without any test 
failures. I'm afraid I can't just make `request` a required parameter, as 
it needs to maintain backwards compatibility. With that said, the current 
function signature of `resolve()` is just... awkward. One option is to 
change it to `resolve(request_or_path, urlconf=None)`, and get the 
path_info from the request if a request is supplied. `resolve()` might 
still be called in a context where there is not request available, so I 
think we need to maintain the ability to resolve just the path. 
 

> *Minor bug:* Given two subdomains, my.example.com and localhost:8000, 
> going to a url using the 'localhost:8000' subdomain that only exists on 
> the 'my' subdomain (ie. http://my.example.com/test3/ exists, but you try 
> to go to http://localhost:8000/test3/), the debug mode 'patterns tried 
> list' is a series of blank lines. See image below:
>

I would've expected at least the regexes here. I'll look into this. 

*​Nice to have:* When attempting to use multiple constraints (ie. list or 
> tuple of constraints), using `RegexPattern` seems to be required when 
> doing pattern matching - otherwise it messes up while calling `construct`. 
> First glance says this is by design? I think it would be nice to still be 
> able to use the old r'' (without wrapping in `RegexPattern`) 
> syntax as well. Not critical, as the multi-constraints is NOT breaking 
> behaviour, just new.
>

Yes, this is more or less by design. While in the case of a single 
constraint it's nice and concise. Once you use a list of constraints you'll 
have to start counting brackets to see if that string is a RegexPattern or 
a parameter to another constraint, or you'll put each constraint on a new 
line and it doesn't matter as much imo. It isn't set in stone, so if you 
have any compelling arguments to change this, let me know. 
 

> *Nice to have:* I realise this isn't likely to happen at all, but it 
> would be nice if when `reverse()` and `{% url %}` are called, it would be 
> good to be able to automatically drop the scheme and host when 
> reconstituting an absolute URL if the scheme and host of the current 
> request match.  However, I'm pretty sure that this is not possible, given 
> the various scenarios in which these methods can be called. Obviously, this 
> is not required, as the resulting URL (with scheme/host or without when 
> matching) will still work regardless!
>

I'll probably implement something like this before it is merged. I've been 
struggling to implement the `URL` class in a way that provided such methods 
in a way that fully conformed RFC 3986, but it was taking up too much of my 
time and I dropped it for now. I'm not sure I'll get it to conform to RFC 
3986, but I'll surely write some basic implementation to output urls 
relative to the current host and scheme. Of course it won't cover 100% of 
the cases as the "current" host and scheme are not always available, but in 
the basic cases it should be doable. 

Op vrijdag 10 juli 2015 15:21:36 UTC+2 schreef Tim Graham:
>
> Marten, did you consider putting the new API in `django.urls` instead of 
> `django.core.urls`? I don't need there's a need to namespace it under 
> "core", but others might be able to confirm the philosophy behind this.
>

That's certainly worth considering, in fact, I quite like the idea. If 
there are no objections (philosophical or otherwise) I'll move it to 
`django.urls`. 

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/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/a8174a99-2364-4e52-9368-320e94810c62%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: URL dispatcher API

[Marten Kenbeek]

Hey,

I've been working on (re)building the new dispatcher API from the ground up 
at https://github.com/knbk/django/tree/dispatcher_api. 
As of this week it is passing the complete test suite, and I've replaced 
all occurrences of the old dispatcher with the new one. It's 
not 100% ready but it's getting closer. 

The API hasn't changed too much since my last post. After several 
iterations of the URL class, I went back to simply passing the remaining 
path, with the request as a second parameter. The URL class is now solely 
used in reverse() to reconstruct urls. After the View class has been 
merged in Resolver, I've now split Resolver into Resolver and 
ResolverEndpoint, with common functionality in BaseResolver. App names are 
now resolved to namespaces before calling resolver.search(), as the former 
method of resolving them on the fly posed some problems. 

The biggest issue still at hand is that `RegexPattern.construct()` can't 
always reliably determine which `normalized_pattern` to use if  there is 
more than one option. Using `reversed(self.normalized_patterns)` and trying 
all options from most specific to least specific might be enough 
in practice, but it can also be a cause for hard-to-debug problems. I 
haven't given it much attention yet, but I'm looking for an efficient way 
to 
solve this while maintaining a simple API for `construct()`. 

As always, feedback is welcome.

Marten

Op zondag 24 mei 2015 02:13:31 UTC+2 schreef Marten Kenbeek:
>
> Hi all,
>
> Thanks for the feedback. It's good to know I'm on the right track.
>
> I've pushed another commit that addresses most of the concerns raised 
> here. I've updated the example gist as well. 
>
> I'm also curious to learn more about this "set of decorators" that can 
>> be applied to a view by the url resolver - that feature doesn't seem to 
>> be documented in the linked gist or in the API example. 
>
>
> The idea is that sometimes, sets of views require a certain decorator - 
> i.e. the whole admin uses the staff_member_required decorator (and some 
> more stuff). This feature allows you to specify a list of decorators to 
> apply to a whole section of your site, and prevents a lot of code 
> duplication and the risk of missing a decorator somewhere (for example, if 
> you add additional views to the admin, you'll have to decorate them 
> yourself). 
>
> I am not at all sure that it is a good idea to mix layers in this 
>> suggested way, or what specific use case it improves. If the argument 
>> that a constraint matches is an integer, why is it (in practice) useful 
>> for the constraint to know that this integer is supposed to be the ID of 
>> some particular model class? Why is it good for that constraint, when 
>> reversing, to be able to take a model instance instead of an ID? To me 
>> this sounds like the sort of implicit type coercion that makes APIs 
>> harder to use and understand, not easier. 
>
>
> The constraints should be very explicit about what they accept. The 
> model_or_pk pattern is used all over Django, so I don't think it's *that* 
> much of a problem. I see two main advantages: you can easily change the 
> field in the url (e.g. from id to slug/uuid), and the regex pattern can be 
> inferred from the field. The latter certainly is more friendly to 
> beginners. But well, maybe it isn't my best idea ever. It doesn't have to 
> be in core, it's easy enough to create in a third-party app if people want 
> it.
>
> 1) The interplay between URL and Constraint is unclear. 
>
>
> I moved the constraints out of the URL class, so that the Constraint knows 
> about the URL. The URL is not strictly immutable, but I think that's 
> difficult to avoid. Now, `Resolver.match()` is a more-or-less atomic 
> operation that clones the old url, manipulates the clone through 
> `Constraint.match()` and returns it. After that, the URL shouldn't change. 
> When reversing, the URL is created from a set of constraints in one go. 
>
> The URL class is something I'm still working on. I particularly like the 
> option to build an url relative to the current request. That should 
> gracefully handle subdomains, and it can easily be extended to handle 
> schemes etc. It might not be needed in resolve(), though. I would like the 
> request object available, but that could be passed as a separate argument. 
> I don't think the pattern of cutting of the matching part should really be 
> extended to the host, so it would only need the path and request 
> parameters. 
>
> 2) The Resolver / View API can be simplified. 
>
>
> I was afraid that Resolver.resolve() would become too complicated, but 
> I've managed to keep it simple. I've brought back the ResolverMatch class 
> (which I would need for backwards compatibil

Re: URL namespaces

[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 
>>>> <https://www.google.com/url?q=https%3A%2F%2Fcode.djangoproject.com%2Fticket%2F21927%23comment%

Re: URL namespaces

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

Re: URL namespaces

[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 
>> <https://www.google.com/url?q=https%3A%2F%2Fcode.djangoproject.com%2Fticket%2F21927%23comment%3A9=D=1=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 hel

Re: URL namespaces

[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

[Marten Kenbeek]

me 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 namespaces. The second case does not in any way enforce 
>>>> that multiple deployments in fact have the same application namespace. 

Re: URL namespaces

[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')),
>> ]
>>
>&

URL namespaces

[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/group/django-developers.

Re: URL dispatcher API

[Marten Kenbeek]

 be used once in 
a single urlconf, and the view has to solve problems that the url 
dispatcher should handle. With the second approach the old url dispatcher 
makes it virtually impossible to make runtime changes. I'm sure that's not 
all ;)

I will probably create a separate thread to discuss the mess that is url 
namespaces. 

Marten

Op zondag 17 mei 2015 16:27:32 UTC+2 schreef Tino de Bruijn:
>
> Hi Marten,
>
> I am a little less acquainted with the internals of resolving and 
> reversing than the people above, and I must say that I have a hard time 
> understanding your new syntax when looking at the example gist. That might 
> also have to do with the indentation style. Would the (blog_detail, 
> blog_index) -> blog_resolver -> patterns be the replacement of the 
> urlpatterns below? Because I think the latter is a lot more readable. 
>
> Do you have an example of something that is easy to do with this API that 
> is hard or impossible with the current API?
>
> Thanks,
>
>
> Tino
>
> On Sun, May 17, 2015 at 2:35 PM, Anssi Kääriäinen <akaa...@gmail.com 
> > wrote:
>
>> One important thing to keep in mind is reverse() performance.
>>
>> While it isn't important that the prototype is fast, making sure the API 
>> doesn't limit performance for the most important operations must be checked.
>>
>> At least my experience is that URL resolving performance isn't that 
>> important, but having fast reverse() is really important. The reason is 
>> that single request typically has just one resolve operation, and thus the 
>> amount of time used for resolving tends to end up in the noise. But large 
>> list/table views can have up to thousands of reverse() operations, when 
>> there are multiple urls per row.
>>
>> I believe we can have clean, fast and extensible URL resolving system, so 
>> I'm not too worried about this. Still, lets make sure performance won't be 
>> a problem.
>>
>>  - Anssi
>>
>>
>> On Monday, May 4, 2015, Marten Kenbeek <marte...@gmail.com > 
>> wrote:
>>
>>> Hi all,
>>>
>>> I'd like to discuss the API I'm proposing for the new url dispatcher I'm 
>>> working on. I'll try to explain the API and some of the rationale behind it.
>>>
>>> There is a working proof-of-concept at 
>>> https://github.com/knbk/django/tree/url_dispatcher. Currently, all the 
>>> names are chosen as not to conflict with the old implementation. A short 
>>> description can be found at 
>>> https://gist.github.com/knbk/96999abaab4ad4e5f8f9, which also contains 
>>> a link to an example url configuration. 
>>>
>>> My main goal was to create a simple, extensible API. In the old API, 
>>> about 90% of the work was done in the `resolve()`, `_reverse_with_prefix()` 
>>> and `populate()` of the RegexURLResolver. This made for a tightly coupled 
>>> design that was almost impossible to extend.
>>>
>>> My starting point was the RegexURLResolver and RegexURLPattern. Both 
>>> have a regex constraint that can match the current path and extract 
>>> arguments. The former can then pass the remainder of the path to its list 
>>> of resolvers and patterns; the latter can return a ResolverMatch containing 
>>> the callback specified by that pattern. I've kept this distinction, with 
>>> the `Resolver` and `View` classes. The change of name from `Pattern`  to 
>>> `View` is because the `View` object has a bit more logic that determines 
>>> how the view is called. It is a thin, callable wrapper that can optionally 
>>> decorate the actual view function with a set of decorators passed down from 
>>> parent resolvers, and when overwritten, can do some more processing before 
>>> or after the view function is called. 
>>>
>>> The hard-coded dependence on a regex pattern has been abstracted to a 
>>> Constraint object. Each Resolver and View has a (set of) Constraint 
>>> object(s), that can match the current url and extract arguments. A 
>>> RegexPattern that simply mimics the old behaviour will be available, but 
>>> other implementations, such as a Constraint that matches the request's host 
>>> or method, are easily provided. A Constraint can also reverse a set of 
>>> arguments to a partial url. That means that the full set of constraints 
>>> used to match an url to a view, together with a suitable set of arguments, 
>>> can be reversed to the url itself.
>>>
>>> The main strength of a Constraint is that it can contain very specific 
>>> logic about its arguments.

URL dispatcher API

[Marten Kenbeek]

Hi all,

I'd like to discuss the API I'm proposing for the new url dispatcher I'm 
working on. I'll try to explain the API and some of the rationale behind it.

There is a working proof-of-concept 
at https://github.com/knbk/django/tree/url_dispatcher. Currently, all the 
names are chosen as not to conflict with the old implementation. A short 
description can be found 
at https://gist.github.com/knbk/96999abaab4ad4e5f8f9, which also contains a 
link to an example url configuration. 

My main goal was to create a simple, extensible API. In the old API, about 
90% of the work was done in the `resolve()`, `_reverse_with_prefix()` and 
`populate()` of the RegexURLResolver. This made for a tightly coupled 
design that was almost impossible to extend.

My starting point was the RegexURLResolver and RegexURLPattern. Both have a 
regex constraint that can match the current path and extract arguments. The 
former can then pass the remainder of the path to its list of resolvers and 
patterns; the latter can return a ResolverMatch containing the callback 
specified by that pattern. I've kept this distinction, with the `Resolver` 
and `View` classes. The change of name from `Pattern`  to `View` is because 
the `View` object has a bit more logic that determines how the view is 
called. It is a thin, callable wrapper that can optionally decorate the 
actual view function with a set of decorators passed down from parent 
resolvers, and when overwritten, can do some more processing before or 
after the view function is called. 

The hard-coded dependence on a regex pattern has been abstracted to a 
Constraint object. Each Resolver and View has a (set of) Constraint 
object(s), that can match the current url and extract arguments. A 
RegexPattern that simply mimics the old behaviour will be available, but 
other implementations, such as a Constraint that matches the request's host 
or method, are easily provided. A Constraint can also reverse a set of 
arguments to a partial url. That means that the full set of constraints 
used to match an url to a view, together with a suitable set of arguments, 
can be reversed to the url itself.

The main strength of a Constraint is that it can contain very specific 
logic about its arguments. For example, a Constraint may know that it 
resolves to a Model's primary key. If it then receives a Model instance of 
that particular type, it will know how to reverse that model instance to a 
valid string-based partial url, so that it can later be resolved to match 
the same object. It could also e.g. infer the regex pattern from the 
field's type. 

There's one final piece to the puzzle: the URL object. This is the state of 
the url in the process of resolving or reversing an url. It's a two-way 
street: when resolving, it starts out as a full path, and the Constraints 
chip away at the path, while the set of constraints and extracted argument 
grows. When reversing, it starts out as a set of constraints and arguments, 
and reconstructs the partial urls from those constraints and arguments 
until a full url path is reconstructed. It shifts some of the logic from 
the Resolver to the URL, so that it is easier to extend the Resolver. It is 
also a simple container that allows any Constraint access to the full 
request. Last but not least, it allows to dynamically build an url against 
the current request. This is useful if e.g. a constraint matches a 
different subdomain than the current request, so that a link automatically 
points to the right domain. 

I'm looking forwards to your feedback.

Thanks,
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/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3cb36c11-16ee-4702-92a3-f9afb177bbca%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Must a Django Database Support Migrations?

[Marten Kenbeek]

Hi Marcin,

I don't believe it's some sort of ethos that got Andrew to collaborate with 
Django and create the current migration framework: It's the fact that 
pretty much everyone who used Django also used South that convinced Andrew 
to recreate its behaviour as a core Django app.

I'm not saying that Liquibase isn't useful -- I haven't really investigated 
the project you're pitching. I'm simply saying that migrations are 
considered a necessary feature for any decently-sized project, and 
currently Django's way of supplying this is its core migration framework. 
If you are able to patch Django's behaviour to not depend on its core 
migration framework, but to depend on a pluggable migration backend, I'm 
sure everyone will be very happy to see that as a Django core feature. But 
as is currently stand, it's a lot of work to recreate the Django migration 
framework as a pluggable backend, and it's not worth it if there isn't a 
tightly integrated migration framework that picks up the changes to Django 
models. 

It should still be quite easy to use Liquibase for your database 
management: simply set your models as `managed=False`, and you'll have full 
control over your database. You'll simply have to keep your Django models 
up-to-date to use them. 

With king regards,
Marten

Op maandag 27 april 2015 02:33:44 UTC+2 schreef Marcin Nowak:
>
> Hi. 
>
> Before v1.7 there were no migrations and Django was quite useful (for 
> example with South as an 3rd party app). What happened then?
>
> I think that migrations can be decoupled from core and you just don't want 
> to do this for some unknown reason. 
> I'll prepare a pull request if it will not be dropped because of some kind 
> of ethos.
>
> BR
> Marcin
>
> On Saturday, April 18, 2015 at 5:03:39 AM UTC+2, Markus Holtermann wrote:
>>
>> Hey Marvin, 
>>
>> Unfortunately this won't work as a few migration components are tightly 
>> integrated into remaining parts of Django, e.g. the field deconstruction. 
>>
>> /Markus 
>>
>> On April 18, 2015 3:35:09 AM GMT+02:00, Marcin Nowak <
>> marcin@gmail.com> wrote: 
>> >Hi. 
>> >Just give people possibility to disable migrations functionality 
>> >(remove it 
>> >from core and put as a contrib app, if needed). 
>> > 
>> >/BR 
>> >Marcin 
>> > 
>> > 
>> >On Thursday, January 22, 2015 at 8:09:01 PM UTC+1, Andrew Godwin wrote: 
>> >> 
>> >> Aha! Then, I would suggest redesigning MigrationRecorder to only make 
>> >the 
>> >> table when an actual recording operation is done, and have it swallow 
>> >the 
>> >> table not existing as "no migrations applied" the rest of the time, 
>> >if 
>> >> people think that seems sensible. 
>> >> 
>> >> Andrew 
>> >> 
>> >> On Thu, Jan 22, 2015 at 10:44 AM, Markus Holtermann < 
>> >> in...@markusholtermann.eu > wrote: 
>> >> 
>> >>> Hey, 
>> >>> 
>> >>> as soon as the MigrationRecorder is used there is a call to 
>> >>> "ensure_schema" that forces the creation of the migration table. The 
>> > 
>> >>> runserver command (among others?) checks for unapplied migrations 
>> >and thus 
>> >>> creates the migration table. 
>> >>> 
>> >>> /Markus 
>> >>> 
>> >>> On Wednesday, January 21, 2015 at 12:36:47 AM UTC+1, Andrew Godwin 
>> >wrote: 
>>  
>>  Hi Ivan, 
>>  
>>  I'm not sure what you're asking here - are you asking to have a way 
>> >to 
>>  not have Django create the migrations recording table? I was under 
>> >the 
>>  impression that it was only created when migrate was run (at least, 
>> >that 
>>  was my original intention) so if you're managing your own schema 
>> >just don't 
>>  run migrate. Is there something else that's not working right, or 
>> >is that 
>>  being made too optimistically? 
>>  
>>  Andrew 
>>  
>>  On Tue, Jan 20, 2015 at 2:44 PM, Ivan VenOsdel  
>> >wrote: 
>>  
>> > From Andrew: "The only extra thing it would achieve is not having 
>> > Django record migrations in the django_migrations table" 
>> > 
>> > The Django Users thread on how to keep this table from being 
>> >created 
>> > seemed to result in the 'solution' being either to stay with 1.6 
>> >or comment 
>> > the relevant lines in the Django code base. Is there really no 
>> >other way? 
>> > 
>> > I love the new migrations facilities in Django 1.7 and was a 
>> > contributor to the kickstarter but please keep in mind that many 
>> >legacy DB 
>> > projects are not avoiding migrations because they want to. IMO 
>> >it's usually 
>> > because they are forced to for some (usually political) reason 
>> >where they 
>> > don't have control over the schema. Forcing a perpetually empty 
>> > django_migrations table pretty much locks out those users. 
>> > 
>> > I see that people are worried about encouraging the non-use of 
>> > migrations but might I suggest following the Zen of Python and 
>> >making 
>> > migrations the "one obvious way to do it" and 

Re: Extending the URL Dispatcher (GSoC 2015 proposal)

[Marten Kenbeek]

I've updated my proposal with a few more changes. Specifically, I've added 
a small part about reversing namespaced urls without specifying the 
namespace. In some circumstances this can provide looser coupling between 
third-party apps and arbitrary, reversible objects. It can also ease the 
transition from non-namespaced urls to namespaced urls by providing an 
intermediate step where an url name can match both. This step is absolutely 
necessary if third-party apps want to switch to namespaced urls but also 
want to provide backwards compatibility with previous versions. 

Additionally, I've included a timeline. Not my strongest point, but I think 
it's a decent guideline of what I'll be working on during the project. 

The up-to-date proposal can be found 
at 
http://www.google-melange.com/gsoc/proposal/public/google/gsoc2015/knbk/5629499534213120.
 
Any additional feedback before (or after) the deadline is greatly 
appreciated. If something in the proposal is not clear, let me know and 
I'll try to explain it better in my proposal. 

There are still a few design decisions to make between now and the start of 
GSoC, but I'll probably create a separate thread for those. 

Thanks,
Marten

Op maandag 2 maart 2015 17:57:24 UTC+1 schreef Marten Kenbeek:
>
> Hey all,
>
> I'm working on a proposal to extend the URL dispatcher. Here, I'd like to 
> provide a quick overview of the features I propose. 
>
> I'd like to:
> - Allow matching based on request attributes such as the subdomain or 
> protocol, and business logic such as the existence of a database object.
> - Make middleware configurable for a subset of views. It should be easy to 
> add, reorder or replace middleware at any level in the (currently 
> recursive) matching algorithm. 
> - Provide conventions for common patterns, such as an easy-to-configure 
> URL router for all generic Model views. For generic views, this should be a 
> one-liner. For custom views and other non-default options, this should 
> still be relatively easy to configure compared to writing out all patterns. 
>
> In the process, I'd like to formalize some classes used in the dispatcher. 
> Currently, the RegexURLPattern and RegexURLResolver classes provide most of 
> the functionality of the URL dispatcher. By abstracting these classes, and 
> factoring out the loading mechanism and some other internals, I hope to 
> provide an extensible dispatching framework for third-party apps.
>
> The full, yet incomplete proposal can be found at 
> https://gist.github.com/knbk/325d415baa92094f1e93 if you want more 
> details. It currently contains a slightly more in-depth discussion of the 
> current dispatcher and the proposed features, and a start on the redesign 
> of the dispatcher. 
>
> I'm mostly looking for some feedback on the general direction of these 
> features, though any feedback is welcome. I'm still working on it, so 
> details might change based on new insights.
>
> Thanks,
> 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/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b0df1d73-5ce4-4417-85f5-214faca6c3fc%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Extending the URL Dispatcher (GSoC 2015 proposal)

[Marten Kenbeek]

I came across an app named django-url-namespaces[1]. It provides support 
for declarative style url patterns, very similar to the declarative style 
of models and forms. I'm not particularly for or against this style or the 
current style, but I wanted to put this out here for discussion. It can 
simplify namespaces as well: the app name is nothing more than the class 
name (if not overridden), and the namespace is nothing more than the 
attribute name of the including class (again, if not overridden). This 
project is going towards a more class-based approach anyway, so it might 
fit the new design.

Any strong feelings or convincing arguments for one or the other? I'm 
slightly in favour of the declarative class style, mostly because it's in 
line with many other parts in Django, though there is practically no 
difference between classes and their instances, unlike forms and models. 
You might treat the class as a configuration and an instance as a resolver 
match, though, which would give clear, distinctive semantics to classes and 
instances. The instance would work both for resolving and reversing, as it 
knows all about the view, the url and the arguments. 

Anyway, I have an updated proposal available 
at https://gist.github.com/knbk/cd0d339e1d3fa127cf7a. 

I've intentionally left out a `ContinueResolving` exception. This is 
nothing that cannot easily be implemented with a custom resolver. What is 
harder to implement with a custom resolver is a `StopResolving` 
(`AbortResolving`? need to work on the name) kind of exception, basically a 
`Resolver404` that breaks through its current recursion level. I feel it is 
warranted that this is included in the proposal. Think e.g. of a 
`SubdomainResolver`. You usually don't want to include all the urls for the 
main domain into your subdomain, so this allows you to raise 
`StopResolving` if none of the subdomain urls match. 

[1] https://github.com/fish2000/django-url-namespaces

Op maandag 9 maart 2015 15:01:02 UTC+1 schreef Marten Kenbeek:
>
> After all the feedback I got here and at the sprint, I think the core of 
> my proposal will be the revamping of the current dispatcher and creating a 
> public API. I'll keep in mind the other features and in general the 
> extendibility of the new dispatcher, and if there is time left I'll start 
> implementing some of them.
>
> One interesting note is how content management systems try to work with 
> the url dispatcher. Most systems simply use a catch-all pattern. This often 
> includes custom machinery to resolve and reverse url patterns for pages, 
> blog posts, and other content types or plugins. Django's url dispatcher is 
> completely static in that it doesn't provide any public API to change the 
> url configuration after it has been loaded. This can be problematic with 
> the dynamic nature of CMS's, hence the custom machinery. Bas (bpeschier) 
> had to take it to a new level by routing certain content entries to a 
> custom view. If you want to avoid a "router" view (which is the url 
> dispatcher's job after all), you'd need to dig into the internals of the 
> url dispatcher to have any kind of dynamic updating of the configuration.
>
> I'd like to keep this dynamic nature in mind when designing the new API, 
> and in time implement a public API for this as well (e.g. a simple 
> `register` and `unregister`). This would avoid the need for either a router 
> view or unique url prefixes for each content type as well. It should 
> certainly allow for granular control, I believe reloading the complete url 
> dispatcher can take quite some time (I should probably test that). 
>
> I'm still in doubt on whether I should implement a refactor of url 
> namespaces and middleware. Url namespacing is overly complex and I'm not 
> too sure what the exact goal of the mechanism is. It obviously needs to 
> differentiate multiple instances of the same url configuration, and it is 
> also used to differentiate url configurations as well as to provide a 
> default instance for an url configuration. I'm not too sure what is 
> absolutely needed and what just makes it more complicated than necessary. 
> However, as namespaces are such an integral part of the dispatcher, it is 
> worth looking into and it might be necessary to keep in mind with the new 
> API. 
>
> As for middleware, I'm inclined to only support per-include decorators. 
> Users can always use `decorator_from_middleware` to define middleware for a 
> subset of views. While middleware certainly needs a revamp, I'm not too 
> familiar with its current issues, and I feel this is slightly out of this 
> project's scope. 
>
> On Friday, March 6, 2015 at 5:21:01 PM UTC+1, Tom Christie wrote:
>>
>> > E.g., flask uses a simple `` to match an integer and capture it 
>> in the `id` parameter. Support to 

Re: Extending the URL Dispatcher (GSoC 2015 proposal)

[Marten Kenbeek]

After all the feedback I got here and at the sprint, I think the core of my 
proposal will be the revamping of the current dispatcher and creating a 
public API. I'll keep in mind the other features and in general the 
extendibility of the new dispatcher, and if there is time left I'll start 
implementing some of them.

One interesting note is how content management systems try to work with the 
url dispatcher. Most systems simply use a catch-all pattern. This often 
includes custom machinery to resolve and reverse url patterns for pages, 
blog posts, and other content types or plugins. Django's url dispatcher is 
completely static in that it doesn't provide any public API to change the 
url configuration after it has been loaded. This can be problematic with 
the dynamic nature of CMS's, hence the custom machinery. Bas (bpeschier) 
had to take it to a new level by routing certain content entries to a 
custom view. If you want to avoid a "router" view (which is the url 
dispatcher's job after all), you'd need to dig into the internals of the 
url dispatcher to have any kind of dynamic updating of the configuration.

I'd like to keep this dynamic nature in mind when designing the new API, 
and in time implement a public API for this as well (e.g. a simple 
`register` and `unregister`). This would avoid the need for either a router 
view or unique url prefixes for each content type as well. It should 
certainly allow for granular control, I believe reloading the complete url 
dispatcher can take quite some time (I should probably test that). 

I'm still in doubt on whether I should implement a refactor of url 
namespaces and middleware. Url namespacing is overly complex and I'm not 
too sure what the exact goal of the mechanism is. It obviously needs to 
differentiate multiple instances of the same url configuration, and it is 
also used to differentiate url configurations as well as to provide a 
default instance for an url configuration. I'm not too sure what is 
absolutely needed and what just makes it more complicated than necessary. 
However, as namespaces are such an integral part of the dispatcher, it is 
worth looking into and it might be necessary to keep in mind with the new 
API. 

As for middleware, I'm inclined to only support per-include decorators. 
Users can always use `decorator_from_middleware` to define middleware for a 
subset of views. While middleware certainly needs a revamp, I'm not too 
familiar with its current issues, and I feel this is slightly out of this 
project's scope. 

On Friday, March 6, 2015 at 5:21:01 PM UTC+1, Tom Christie wrote:
>
> > E.g., flask uses a simple `` to match an integer and capture it 
> in the `id` parameter. Support to check for conflicts would be a lot 
> simpler with those patterns. It would definitely be a best-effort feature.
>
> From my point of view, this by itself would make for a really 
> nicely-scoped GSoC project.
> Being able to demonstrate an API that allowed the user to switch to a URL 
> resolver that used that simpler style would be a really, really nice 
> feature,
> and also feels like it might actually be a manageable amount of work.
> This wouldn't *necessarily* need to allow decorator style routing, instead 
> of the current URLConf style, but that might also be a nice addition. 
> Personally though I would consider tackling that as an incremental 
> improvement.
>
> Things I'd be wary of:
>
> * Anything around "continue resolving" exceptions or object inspection 
> during routing, both of which sound like an anti-pattern to me.
> * Method based routing. Feel less strongly about this, but still not 
> convinced that it's a good style.
> * Generic views / model routes / automatic routing. Too broadly defined, 
> and with no clear best answer.
>
> Anyways, interesting stuff Marten, look forward to hearing more.
>
>   Tom
>

-- 
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/0bf15653-717c-4e24-8406-14fa71bf5bf9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Extending the URL Dispatcher (GSoC 2015 proposal)

[Marten Kenbeek]

ly worth taking a look. A revamped framework would 
also allow for an easy extension later on that adds resolvers based on 
this, in case it isn't included initially. 

I'll be at the Django sprint in Amsterdam this Saturday, maybe some 
interesting ideas or insights will roll out. I'll be sure to revisit my 
proposal and this thread afterwards. 

Marten

On Wednesday, March 4, 2015 at 2:37:46 AM UTC+1, Marten Kenbeek wrote:
>
> First of all, thanks for the feedback. There are some good points here 
> that I hadn't thought about.
>
> Behaviour similar to a `ContinueResolving` exception is one of the things 
> I was aiming at. However, I can't see how to raise this in a view while 
> maintaining backwards compatibility with view middleware. E.g. the 
> CsrfViewMiddleware might have bailed out before the first view was 
> executed, while the second view might be csrf exempt. Executing the view 
> middleware twice might cause problems as well.
>
> As for alternate url resolvers: they'll provide the same functions 
> (`resolve` and `reverse`) that are required for any new-style resolvers, so 
> they can be freely mixed and replaced, for example:
>
> urlpatterns = patterns('',
> SubdomainResolver('accounts.', include('some.url.config.urls'), 
> decorators=[login_required]),
> url(r'^$', 'my.view.function'),
> ModelRouter(r'^mymodel/', model=MyModel, field='some_field',
> views = {
> 'detail': MyDetailView.as_view(),
> 'list':   MyListView.as_view(),
> 'edit':   MyUpdateView.as_view(),
> 'new':MyCreateView.as_view(),
> 'delete': None,
> }
> ),
> )
>
> As for the model router: a real-world example can be as simple as this. 
> You can either specify a field and based on the field, `detail` and `edit` 
> will accept only numbers or strings or whatever, or you can specify your 
> own regex that is repeated for all single-object views. The magic is that 
> the model router will automatically reverse the url based on a model or 
> model instance if that model uses a model router, similar to how 
> `get_absolute_url` provides the same functionality for a single view per 
> model.
>
> However, looking at ruby-on-rails and django-rest-framework, routers are 
> tightly coupled with controllers/viewsets, which provide all the views for 
> a specific model. Django's own `ModelAdmin` combines the two, though it is 
> more of a router-and-controller-and-much-more in one. Controllers and 
> viewsets are both tightly coupled with the routers, so it might be 
> warranted that a possible viewset simply implements the `resolve` and 
> `reverse` methods for its contained views.
>
> The `decorators` parameter above also shows what I had in mind in that 
> regard. After looking for concrete examples, I can't seem to think of a 
> proper use case for the same behaviour for middleware, at least not with 
> Django's built-in middleware. For decorators, though, it's a great 
> addition, if there is a proper way to add, reorder or remove inherited 
> decorators (reorder mostly because of csrf_exempt).
>
> That's it for today.
>
> Marten
>
> On Tuesday, March 3, 2015 at 2:56:39 AM UTC+1, Curtis Maloney wrote:
>>
>>
>>
>> On 3 March 2015 at 03:57, Marten Kenbeek <marte...@gmail.com> wrote:
>>
>>> Hey all,
>>>
>>> I'm working on a proposal to extend the URL dispatcher. Here, I'd like 
>>> to provide a quick overview of the features I propose. 
>>>
>>> I'd like to:
>>> - Allow matching based on request attributes such as the subdomain or 
>>> protocol, and business logic such as the existence of a database object.
>>>
>>
>> There was a "continue resolving" sort of exception proposed/implemented 
>> that would obviate this, allowing the logic to remain in views [or view 
>> decorators]... a much simpler solution, IMHO.
>>  
>>
>>> - Make middleware configurable for a subset of views. It should be easy 
>>> to add, reorder or replace middleware at any level in the (currently 
>>> recursive) matching algorithm. 
>>>
>>
>> This has certainly been on the "wanted" list for many years now, however 
>> I expect it would require the middleware re-design that so far has proven 
>> too invasive to land.
>>
>> That said, providing the "new" middleware-as-wrapper interface around url 
>> patterns lists could be a good stepping stone to eventually removing the 
>> existing middleware API.
>>  
>>
>>> - Provide conventions for common patterns, such as an easy-to-configure 
>>> URL router for all ge

Re: Extending the URL Dispatcher (GSoC 2015 proposal)

[Marten Kenbeek]

First of all, thanks for the feedback. There are some good points here that 
I hadn't thought about.

Behaviour similar to a `ContinueResolving` exception is one of the things I 
was aiming at. However, I can't see how to raise this in a view while 
maintaining backwards compatibility with view middleware. E.g. the 
CsrfViewMiddleware might have bailed out before the first view was 
executed, while the second view might be csrf exempt. Executing the view 
middleware twice might cause problems as well.

As for alternate url resolvers: they'll provide the same functions 
(`resolve` and `reverse`) that are required for any new-style resolvers, so 
they can be freely mixed and replaced, for example:

urlpatterns = patterns('',
SubdomainResolver('accounts.', include('some.url.config.urls'), 
decorators=[login_required]),
url(r'^$', 'my.view.function'),
ModelRouter(r'^mymodel/', model=MyModel, field='some_field',
views = {
'detail': MyDetailView.as_view(),
'list':   MyListView.as_view(),
'edit':   MyUpdateView.as_view(),
'new':MyCreateView.as_view(),
'delete': None,
}
),
)

As for the model router: a real-world example can be as simple as this. You 
can either specify a field and based on the field, `detail` and `edit` will 
accept only numbers or strings or whatever, or you can specify your own 
regex that is repeated for all single-object views. The magic is that the 
model router will automatically reverse the url based on a model or model 
instance if that model uses a model router, similar to how 
`get_absolute_url` provides the same functionality for a single view per 
model.

However, looking at ruby-on-rails and django-rest-framework, routers are 
tightly coupled with controllers/viewsets, which provide all the views for 
a specific model. Django's own `ModelAdmin` combines the two, though it is 
more of a router-and-controller-and-much-more in one. Controllers and 
viewsets are both tightly coupled with the routers, so it might be 
warranted that a possible viewset simply implements the `resolve` and 
`reverse` methods for its contained views.

The `decorators` parameter above also shows what I had in mind in that 
regard. After looking for concrete examples, I can't seem to think of a 
proper use case for the same behaviour for middleware, at least not with 
Django's built-in middleware. For decorators, though, it's a great 
addition, if there is a proper way to add, reorder or remove inherited 
decorators (reorder mostly because of csrf_exempt).

That's it for today.

Marten

On Tuesday, March 3, 2015 at 2:56:39 AM UTC+1, Curtis Maloney wrote:
>
>
>
> On 3 March 2015 at 03:57, Marten Kenbeek <marte...@gmail.com 
> > wrote:
>
>> Hey all,
>>
>> I'm working on a proposal to extend the URL dispatcher. Here, I'd like to 
>> provide a quick overview of the features I propose. 
>>
>> I'd like to:
>> - Allow matching based on request attributes such as the subdomain or 
>> protocol, and business logic such as the existence of a database object.
>>
>
> There was a "continue resolving" sort of exception proposed/implemented 
> that would obviate this, allowing the logic to remain in views [or view 
> decorators]... a much simpler solution, IMHO.
>  
>
>> - Make middleware configurable for a subset of views. It should be easy 
>> to add, reorder or replace middleware at any level in the (currently 
>> recursive) matching algorithm. 
>>
>
> This has certainly been on the "wanted" list for many years now, however I 
> expect it would require the middleware re-design that so far has proven too 
> invasive to land.
>
> That said, providing the "new" middleware-as-wrapper interface around url 
> patterns lists could be a good stepping stone to eventually removing the 
> existing middleware API.
>  
>
>> - Provide conventions for common patterns, such as an easy-to-configure 
>> URL router for all generic Model views. For generic views, this should be a 
>> one-liner. For custom views and other non-default options, this should 
>> still be relatively easy to configure compared to writing out all patterns. 
>>
>
> Are you talking about pre-cooked url patterns for the various CBV?  Or 
> plugin routers for groups of CBV?  I'm certainly in favour of some tool 
> that makes it easier to express "common" regex matches [satisfying the 
> "protect from the tedium" rule of frameworks]
>  
>
> In the process, I'd like to formalize some classes used in the dispatcher. 
>> Currently, the RegexURLPattern and RegexURLResolver classes provide most of 
>> the functionality of the URL dispatcher. By abstracting these classes, and 
>> factoring out the loading mecha

Extending the URL Dispatcher (GSoC 2015 proposal)

[Marten Kenbeek]

Hey all,

I'm working on a proposal to extend the URL dispatcher. Here, I'd like to 
provide a quick overview of the features I propose. 

I'd like to:
- Allow matching based on request attributes such as the subdomain or 
protocol, and business logic such as the existence of a database object.
- Make middleware configurable for a subset of views. It should be easy to 
add, reorder or replace middleware at any level in the (currently 
recursive) matching algorithm. 
- Provide conventions for common patterns, such as an easy-to-configure URL 
router for all generic Model views. For generic views, this should be a 
one-liner. For custom views and other non-default options, this should 
still be relatively easy to configure compared to writing out all patterns. 

In the process, I'd like to formalize some classes used in the dispatcher. 
Currently, the RegexURLPattern and RegexURLResolver classes provide most of 
the functionality of the URL dispatcher. By abstracting these classes, and 
factoring out the loading mechanism and some other internals, I hope to 
provide an extensible dispatching framework for third-party apps.

The full, yet incomplete proposal can be found at 
https://gist.github.com/knbk/325d415baa92094f1e93 if you want more details. 
It currently contains a slightly more in-depth discussion of the current 
dispatcher and the proposed features, and a start on the redesign of the 
dispatcher. 

I'm mostly looking for some feedback on the general direction of these 
features, though any feedback is welcome. I'm still working on it, so 
details might change based on new insights.

Thanks,
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/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/1555e45b-8a98-4d36-bc7b-00c7136e782d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Support for DATABASES['default'] = {}

[Marten Kenbeek]

If the docs state that it is a valid setting, then we shouldn't change that 
to maintain compatibility and your ticket is indeed a bug. In that case, it 
makes sense to me not to require the `default` database setting in the 
first place. The error caused in #24394 is a result of looping over all 
available connections, removing this requirement should fix the bug as 
well. 

Looking at just the traceback for this bug, it seems that, without a 
`default` configuration, a `--database` option in the test command is not 
necessary with proper routing in the application, and it wouldn't even make 
sense in a multi-db testing environment. 

On Wednesday, February 25, 2015 at 1:34:34 PM UTC+1, Thomas Recouvreux 
wrote:
>
> Hello,
>
> I have a case where the 'default' database does not have any sense, but I 
> still want the tests to create and manage transactions on other defined 
> databases, so the option `--database` would not help me. I could put 
> something in the default database like in memory sqlite for the tests but 
> it does not feel good to me since this could lead to successful tests but 
> failures on production environment.
>
> Would it be possible to remove the requirement of the 'default' alias ? It 
> seems a bit odd to me that DATABASES = {} and DATABASES = { 'default': {}, 
> 'mysite': { .. } } are valid settings but DATABASES = { 'mysite': { .. } } 
> is not.
>
> Whatever is the final decision, do not forget to update the doc if 
> necessary, since the current doc (
> https://docs.djangoproject.com/en/1.7/topics/db/multi-db/#defining-your-databases)
>  
> states DATABASES = { 'default': {} } is a valid setting.
>
>
>
>
>
>
> On Tuesday, February 24, 2015 at 4:42:52 PM UTC+1, Markus Holtermann wrote:
>>
>> The question I'm asking myself right now: what is a "default" database in 
>> a multi database setup where "default" does not make sense at all? I can 
>> easily think of a case where I have multiple other databases used by other 
>> services where Django provides a dashboard. I don't see any of those 
>> databases being a "default".
>>
>> That said, having an implicit default database that defaults to a dummy 
>> backend doesn't seem too bad. I'd rather see the docs updated in that 
>> regards and fix code that relies on "default" although giving an explicit 
>> database alias isn't complicated. "manage.py test" could get a --database 
>> option (if it doesn't have it already).
>>
>> /Markus
>>
>> On Tuesday, February 24, 2015 at 3:11:08 PM UTC+1, Marc Tamlyn wrote:
>>>
>>> I can't see why it is sensible to support an invalid database 
>>> configuration as the default. If you explicitly want the default to be a 
>>> dummy, you should configure that IMO.
>>>
>>> On 24 February 2015 at 14:04, Marten Kenbeek <marte...@gmail.com> wrote:
>>>
>>>> Which one? :P This was more intended as a question than as a proposal.
>>>>
>>>> I personally prefer a documentation change and strict checking of 
>>>> `default` if and only if the database configuration is not empty. 
>>>> django.db.connection relies on the default connection, and third-party 
>>>> apps 
>>>> which (intentionally or unintentionally) do not support multi-db setups 
>>>> might be using that. I can't think of a scenario where it hurts to have a 
>>>> default database. I'm easily swayed on this matter, though. 
>>>>
>>>> On Tuesday, February 24, 2015 at 2:52:47 PM UTC+1, Marc Tamlyn wrote:
>>>>>
>>>>> In that case your proposal sounds perfectly reasonable.
>>>>>
>>>>> On 24 February 2015 at 13:47, Marten Kenbeek <marte...@gmail.com> 
>>>>> wrote:
>>>>>
>>>>>> In fact, DATABASES={} is a valid configuration and merely sets 
>>>>>> 'default' as a dummy backend. An exception is only explicitly raised if 
>>>>>> you 
>>>>>> supply a non-empty setting that does not include `default`. 
>>>>>>
>>>>>> On Tuesday, February 24, 2015 at 2:43:51 PM UTC+1, Marc Tamlyn wrote:
>>>>>>>
>>>>>>> It would seem more sensible to me to try to support DATABASES={}. 
>>>>>>> There's no reason why a Django site should have to run a database - a 
>>>>>>> microservice using redis or something similar is perfectly reasonable 
>>>>>>> and 
>>>>>>> you could want to use Django for other reasons (e.g. shared templates).
&g

Re: Support for DATABASES['default'] = {}

[Marten Kenbeek]

Which one? :P This was more intended as a question than as a proposal.

I personally prefer a documentation change and strict checking of `default` 
if and only if the database configuration is not empty. 
django.db.connection relies on the default connection, and third-party apps 
which (intentionally or unintentionally) do not support multi-db setups 
might be using that. I can't think of a scenario where it hurts to have a 
default database. I'm easily swayed on this matter, though. 

On Tuesday, February 24, 2015 at 2:52:47 PM UTC+1, Marc Tamlyn wrote:
>
> In that case your proposal sounds perfectly reasonable.
>
> On 24 February 2015 at 13:47, Marten Kenbeek <marte...@gmail.com 
> > wrote:
>
>> In fact, DATABASES={} is a valid configuration and merely sets 'default' 
>> as a dummy backend. An exception is only explicitly raised if you supply a 
>> non-empty setting that does not include `default`. 
>>
>> On Tuesday, February 24, 2015 at 2:43:51 PM UTC+1, Marc Tamlyn wrote:
>>>
>>> It would seem more sensible to me to try to support DATABASES={}. 
>>> There's no reason why a Django site should have to run a database - a 
>>> microservice using redis or something similar is perfectly reasonable and 
>>> you could want to use Django for other reasons (e.g. shared templates).
>>>
>>> Marc
>>>
>>> On 24 February 2015 at 13:38, Marten Kenbeek <marte...@gmail.com> wrote:
>>>
>>>> With recent bug reports (#24332 
>>>> <https://code.djangoproject.com/ticket/24332>, #24298 
>>>> <https://code.djangoproject.com/ticket/24298> and now #24394 
>>>> <https://code.djangoproject.com/ticket/24394>) all caused by setting 
>>>> `DATABASES['default'] = {}`, this makes me wonder if it should be 
>>>> supported 
>>>> at all.
>>>> The documentation states:
>>>>
>>>> The DATABASES 
>>>>> <https://docs.djangoproject.com/en/dev/ref/settings/#std:setting-DATABASES>
>>>>>  setting 
>>>>> must configure a default database; any number of additional databases 
>>>>> may also be specified.[1] 
>>>>
>>>>
>>>> And indeed, if default is not set at all, an error is raised. If 
>>>> default does not provide valid database settings, it is set to use 
>>>> `django.db.backends.dummy`. This will raise a `NotImplementedError` as 
>>>> soon 
>>>> as it is used, but it seems you can work around this quite well and ensure 
>>>> that `default` is barely used. The exceptions to this are reported in the 
>>>> mentioned bug reports, most notably `manage.py test` uses the `default` 
>>>> database. 
>>>>
>>>> Should the documentation be clarified that it not only requires 
>>>> `default` to be set, but that it requires a valid database configuration 
>>>> as 
>>>> well? Or should an empty `default` configuration be valid?
>>>>
>>>> In #24332 and #24398, both fixed now, there was the additional issue 
>>>> that the code was simply not using the database is was supposed to use.
>>>>
>>>> I think this mostly boils down to whether we want the `test` command to 
>>>> support an explicit non-default database, you should be able to manipulate 
>>>> all other code to only use non-default databases, save some similar bugs 
>>>> that might still be present. This would reduce `default` to simple 
>>>> semantics and would probably warrant that it is no longer required if you 
>>>> supply an otherwise valid database configuration. However, I don't see any 
>>>> drawbacks to requiring a valid database setup. At most this would require 
>>>> developers to copy the credentials from the database they are actually 
>>>> using to the `default` setting, but it would also be an incentive to 
>>>> actually use `default` as the default database. 
>>>>
>>>> [1]  https://docs.djangoproject.com/en/dev/ref/settings/#databases
>>>>
>>>> -- 
>>>> 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-develop...@googlegroups.com.
>>>> To post to this group, send email to django-d...@googlegroups.com.
>>>> Visit this group at http://groups.google.com/group/django-developer

Re: Support for DATABASES['default'] = {}

[Marten Kenbeek]

In fact, DATABASES={} is a valid configuration and merely sets 'default' as 
a dummy backend. An exception is only explicitly raised if you supply a 
non-empty setting that does not include `default`. 

On Tuesday, February 24, 2015 at 2:43:51 PM UTC+1, Marc Tamlyn wrote:
>
> It would seem more sensible to me to try to support DATABASES={}. There's 
> no reason why a Django site should have to run a database - a microservice 
> using redis or something similar is perfectly reasonable and you could want 
> to use Django for other reasons (e.g. shared templates).
>
> Marc
>
> On 24 February 2015 at 13:38, Marten Kenbeek <marte...@gmail.com 
> > wrote:
>
>> With recent bug reports (#24332 
>> <https://code.djangoproject.com/ticket/24332>, #24298 
>> <https://code.djangoproject.com/ticket/24298> and now #24394 
>> <https://code.djangoproject.com/ticket/24394>) all caused by setting 
>> `DATABASES['default'] = {}`, this makes me wonder if it should be supported 
>> at all.
>> The documentation states:
>>
>> The DATABASES 
>>> <https://docs.djangoproject.com/en/dev/ref/settings/#std:setting-DATABASES> 
>>> setting 
>>> must configure a default database; any number of additional databases 
>>> may also be specified.[1] 
>>
>>
>> And indeed, if default is not set at all, an error is raised. If default 
>> does not provide valid database settings, it is set to use 
>> `django.db.backends.dummy`. This will raise a `NotImplementedError` as soon 
>> as it is used, but it seems you can work around this quite well and ensure 
>> that `default` is barely used. The exceptions to this are reported in the 
>> mentioned bug reports, most notably `manage.py test` uses the `default` 
>> database. 
>>
>> Should the documentation be clarified that it not only requires `default` 
>> to be set, but that it requires a valid database configuration as well? Or 
>> should an empty `default` configuration be valid?
>>
>> In #24332 and #24398, both fixed now, there was the additional issue that 
>> the code was simply not using the database is was supposed to use.
>>
>> I think this mostly boils down to whether we want the `test` command to 
>> support an explicit non-default database, you should be able to manipulate 
>> all other code to only use non-default databases, save some similar bugs 
>> that might still be present. This would reduce `default` to simple 
>> semantics and would probably warrant that it is no longer required if you 
>> supply an otherwise valid database configuration. However, I don't see any 
>> drawbacks to requiring a valid database setup. At most this would require 
>> developers to copy the credentials from the database they are actually 
>> using to the `default` setting, but it would also be an incentive to 
>> actually use `default` as the default database. 
>>
>> [1]  https://docs.djangoproject.com/en/dev/ref/settings/#databases
>>
>> -- 
>> 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-develop...@googlegroups.com .
>> To post to this group, send email to django-d...@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/756eefc2-1baf-4898-91bf-86d378a27b04%40googlegroups.com
>>  
>> <https://groups.google.com/d/msgid/django-developers/756eefc2-1baf-4898-91bf-86d378a27b04%40googlegroups.com?utm_medium=email_source=footer>
>> .
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>

-- 
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/d8373cb4-27c8-40ea-85b0-c144530b1180%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Support for DATABASES['default'] = {}

[Marten Kenbeek]

With recent bug reports (#24332 
, #24298 
 and now #24394 
) all caused by setting 
`DATABASES['default'] = {}`, this makes me wonder if it should be supported 
at all.
The documentation states:

The DATABASES 
>  
> setting 
> must configure a default database; any number of additional databases may 
> also be specified.[1] 


And indeed, if default is not set at all, an error is raised. If default 
does not provide valid database settings, it is set to use 
`django.db.backends.dummy`. This will raise a `NotImplementedError` as soon 
as it is used, but it seems you can work around this quite well and ensure 
that `default` is barely used. The exceptions to this are reported in the 
mentioned bug reports, most notably `manage.py test` uses the `default` 
database. 

Should the documentation be clarified that it not only requires `default` 
to be set, but that it requires a valid database configuration as well? Or 
should an empty `default` configuration be valid?

In #24332 and #24398, both fixed now, there was the additional issue that 
the code was simply not using the database is was supposed to use.

I think this mostly boils down to whether we want the `test` command to 
support an explicit non-default database, you should be able to manipulate 
all other code to only use non-default databases, save some similar bugs 
that might still be present. This would reduce `default` to simple 
semantics and would probably warrant that it is no longer required if you 
supply an otherwise valid database configuration. However, I don't see any 
drawbacks to requiring a valid database setup. At most this would require 
developers to copy the credentials from the database they are actually 
using to the `default` setting, but it would also be an incentive to 
actually use `default` as the default database. 

[1]  https://docs.djangoproject.com/en/dev/ref/settings/#databases

-- 
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/756eefc2-1baf-4898-91bf-86d378a27b04%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Signature of the allow_migrate database router.

[Marten Kenbeek]

Loïc, the model_name is indeed the lower-case version of the 
object_name: 
https://github.com/django/django/blob/master/django/db/models/options.py#L203

On Wednesday, February 18, 2015 at 2:07:28 PM UTC+1, Loïc Bistuer wrote:
>
> Individual routers don't have a base class but we can add it to the master 
> router `django.db.router`. I've updated the PR with a 
> `router.allow_migrate_model` method. I don't know if we should document 
> this just yet. 
>
> Internally we now treat anything other than `app_label` as a hint, but I 
> guess we should still document `model_name=None` as part of the official 
> signature. 
>
> Speaking about `model_name`, it seems to be the lowercase version of 
> `_meta.object_name` (which is basically __name__). I didn't investigate 
> further, can you confirm that `_meta.model_name` is what we want? 
>
> > On Feb 18, 2015, at 15:41, Aymeric Augustin <
> aymeric@polytechnique.org > wrote: 
> > 
> > 2015-02-18 6:34 GMT+01:00 Loïc Bistuer  >: 
> > Another option would be to make the signature `allow_migrate(self, db, 
> app_label, model_name=None, **hints)` and to put the model class in the 
> hints dict, the same way we pass `instance` as hint to the other routers. 
> > 
> > Yes, that's what I wanted to suggest after Andrew confirmed my suspicion 
> that we have to account for historical models. 
> > 
> > Perhaps the following convenience function can help factoring out the 
> arguments in the common case: 
> > 
> > def allow_migrate_model(router, db, model): 
> > return router.allow_migrate(db, model._meta.app_label, 
> model._meta.model_name, model=model) 
> > 
> > (This isn't a router method because there isn't a base class for 
> routers.) 
> > 
> > -- 
> > 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-develop...@googlegroups.com . 
> > To post to this group, send email to django-d...@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-7mVJpobigSdmEQt1NdkXOyjDWdPoEZJz4TVdVDeG9g%2BS2w%40mail.gmail.com.
>  
>
> > For more options, visit https://groups.google.com/d/optout. 
>
>

-- 
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/9e07c5d3-7706-4873-bb7a-9224af94c35f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: status of 1.8 release blockers

[Marten Kenbeek]

@Tim I believe it's quite easy actually, I've fixed my original patch and 
it's just waiting to be reviewed. It's available at 
https://github.com/django/django/pull/4071. However, it's a very rare bug 
and I only encountered it in the tests when working on another issue, not 
in any real project. Not really worth delaying the beta I think, I'm 
getting the feeling you guys are busy enough as it is. 

Op dinsdag 17 februari 2015 18:20:22 UTC+1 schreef Tim Graham:
>
> If it's the only release blocker left and the fix is difficult, I think we 
> could issue a beta release without it. I think the worst case is that 
> you'll get some duplicate bug reports from testers.
>
> On Tuesday, February 17, 2015 at 10:06:27 AM UTC-5, Markus Holtermann 
> wrote:
>>
>> So, it turns out that #24291 (Migrations fail with unused swappable 
>> model)  only occurs when a 
>> swapped out model doesn't define an explicit model manager. Since auth.User 
>> is the only model that is officially supported for "swappiness" and it 
>> defines an explicit UserManager this isn't really a problem in my opinion. 
>> Since the whole swappable model API is internal API anyway, I don't want to 
>> rule this issue a release blocker. If projects in the wild use the 
>> swappable model API they can get around the issue by adding `objects = 
>> models.Manager()` to that model.
>>
>> Tim, Russ, thoughts 'bout that?
>>
>> /Markus
>>
>> On Tuesday, February 17, 2015 at 1:05:37 AM UTC+1, Tim Graham wrote:
>>>
>>> Here's the status on blockers. Given the work that remains, I don't 
>>> foresee the beta release happening earlier than Wednesday, but the next 
>>> update of this thread will be tomorrow.
>>>
>>> #24324  Crashes when 
>>> project path or path to Django install contains a non-ascii character 
>>> 
>>> Owner: Tim
>>> Status: Patch needs review
>>>
>>>
>>> #24351    RunPython/RunSQL 
>>> operations in contrib migrations and multi-db routers. 
>>> Owner: Loic
>>> Status: API design decision needed. Time permitting, Loic will write to 
>>> the mailing list on this tomorrow.
>>>
>>> #24328  The new 
>>> Options._get_fields() method needs to be cleaned up 
>>> 
>>> Owner: Anssi
>>> Status: Anssi still reviewing the patch; decision may be needed on 
>>> backwards compatibility of get_fields().
>>>
>>> #24343  UUID foreign key 
>>> accessor returns inconsistent type 
>>> Owner: Marc/Josh/Shai
>>> Status: Josh working on cleaning up the patch. Review/feedback from 
>>> Anssi requested.
>>>
>>> #24291 
>>> 
>>>  Migrations 
>>> fail with unused swappable model 
>>> 
>>> Owner: Markus
>>> Status: Patch looks good to me; Markus to review & commit tomorrow.
>>>
>>> On Monday, February 16, 2015 at 11:12:46 AM UTC-5, Tim Graham wrote:

 There are still quite a few unresolved issues, so the beta release 
 won't happen today. I'll send an update at the end of the day with the 
 status of the remaining issues.

 On Friday, February 13, 2015 at 1:30:44 PM UTC-5, Markus Holtermann 
 wrote:
>
> Hey folks,
>
> I looked into the issues #24225, #24264 and #24282 and have a working 
> pull request ready for review: 
> https://github.com/django/django/pull/4097
>
> The essential change in the pull request is the way how the set of 
> models that needs to be rerendered is constructed. Instead of naively 
> only 
> resolving one level of relations (Only rerender B and C if C changed: A 
> --> 
> B --> C) the new approach recursively checks for relational fields 
> (forwards and reverse relations (e.g. ForeignKey and ManyToManyRel)) as 
> well as inheritance.
>
> This approach boils down to the following behavior:
>
> If a completely standalone model (no incoming or outgoing relations 
> and no subclasses and only a directed subclass of models.Model) changes, 
> only this model will be rerendered, which is the best case scenario. If 
> every model is somehow related (directly or through other models) to 
> every 
> other model, e.g. through the user model, changing one model from that 
> set 
> will require *all* models to be rerendered, which is the worst case 
> scenario and results in the similar behavior as 1.7.
>
> To get that pull request into the 1.8 beta I ask everybody to take a 
> look and try it out on their projects. Especially if 1.8 alpha 1 

Re: Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

Hi Loïc,

The implementation itself no longer depends on locks. I wish it was as 
simple as form fields, but the idea is the same. The first commit 
(https://github.com/knbk/django/commit/7ac5b58587ea2a153766d1601965734731609cdf)
 
in fact leaves out the lock feature. It's still an extra option in the 
second commit, to explicitly lock otherwise replaceable fields. I still 
think it's a good addition, but if you and others think it's not, I'll 
happily leave it out. 

Slightly (but not entirely) unrelated:
At the moment, a lot of clashing fields don't throw an error in 
`ModelBase.__new__`, but rather return an error in `Model.check`. With 
backwards-compatibility in mind, what's the policy on changing this to 
throw an error in `ModelBase.__new__` instead? In both cases the models 
don't work, but it would change where and how the error is "raised". 
There's hardly any consistency either: if a new field clashes with a 
directly inherited field, it raises a `FieldError`, but when the clashing 
field is in a grandparent, or a directly inherited field clashes with 
another inherited field, there's no exception, just an error in 
`Model.check`. 

I noticed this because I needed a lot of checks in my original patch to let 
some errors fall through and be caught by `Model.check` instead. I wouldn't 
mind looking into ways to refactor this, probably for a brand-new, 
unrelated patch, but I'm a bit hesitant about the errors. If we have to 
maintain the exact current behaviour, those extra checks for fall-through 
errors would more or less defeat the purpose of refactoring in the first 
place. 

Marten

Op maandag 16 februari 2015 16:26:30 UTC+1 schreef Loïc Bistuer:
>
> Hi Marten, 
>
> As mentioned in my previous email I don't recommend an implementation 
> based on locks. 
>
> Please look at how Form and ModelForm support inheritance and shadowing 
> without it. 
>
> -- 
> Loïc 
>
> > On Feb 16, 2015, at 21:35, Marten Kenbeek <marte...@gmail.com 
> > wrote: 
> > 
> > Okay, as things were getting more and more complicated I sat down for a 
> while to get back to the essence of this patch. The result [1] is much, 
> much cleaner and works perfectly. I'll start working on documentation as 
> well. 
> > 
> > [1] https://github.com/knbk/django/tree/ticket_24305_alternative 
> > 
> > Op vrijdag 13 februari 2015 15:28:53 UTC+1 schreef Marten Kenbeek: 
> > Hi Loïc, 
> > 
> > Thanks for the response. I will change my code to generate errors in the 
> case of abstract fields shadowing concrete fields. 
> > 
> > At the moment, the locking mechanism of fields is pretty much the core 
> of this patch. It is explicitly set to `True` when fields are added to a 
> concrete model, and it allows to easily check if a field can be changed. 
> Without it, a proper, clean way would be needed to check the origin of the 
> field. If for example an abstract model inheriting from a concrete model 
> adds some fields, you should be able to shadow the new fields, but not the 
> inherited fields. Making it internal would be clunky with the way cloning 
> and serializing currently works. 
> > 
> > I'm not too content with this anyway. As the default has to be `False` 
> (for an opt-in behaviour for abstract fields, rather than an opt-out 
> behaviour), this unnecessarily clutters migrations and other serialized 
> values with `locked=True` for concrete classes. However, it's not trivial 
> to consistently determine if a field is in any way inherited from a 
> concrete class. I will look into a clean method to determine this. 
> > 
> > I don't think we should remove it entirely, unless we provide another 
> mechanism to prevent field changes for some abstract fields. E.g. 
> django-polymorphic completely depends on the `polymorphic_ctype` field. 
> That might be a rare name to use, but it shows that some code does depends 
> on abstract fields. With the locking mechanism, we move the responsibility 
> of ensuring the right behaviour of that field to the package developer 
> rather than the website developer. I think a fail hard, fail fast approach 
> is better for these situations than going along with the possibility of 
> subtle or not-so-subtle bugs. 
> > 
> > Op donderdag 12 februari 2015 17:59:58 UTC+1 schreef Loïc Bistuer: 
> > Hi Marten, 
> > 
> > Sorry for the late response, it's been a busy week. Thanks for working 
> on this, I'll try to have a look at your branch this weekend. 
> > 
> > In the meantime I'll leave some comments below. 
> > 
> > > On Feb 12, 2015, at 22:43, Marten Kenbeek <marte...@gmail.com> wrote: 
> > > 
> > > Current status: 
> > > • Models in general seem to be working. 
> > >

Re: Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

Okay, as things were getting more and more complicated I sat down for a 
while to get back to the essence of this patch. The result [1] is much, 
much cleaner and works perfectly. I'll start working on documentation as 
well.

[1] https://github.com/knbk/django/tree/ticket_24305_alternative

Op vrijdag 13 februari 2015 15:28:53 UTC+1 schreef Marten Kenbeek:
>
> Hi Loïc,
>
> Thanks for the response. I will change my code to generate errors in the 
> case of abstract fields shadowing concrete fields. 
>
> At the moment, the locking mechanism of fields is pretty much the core of 
> this patch. It is explicitly set to `True` when fields are added to a 
> concrete model, and it allows to easily check if a field can be changed. 
> Without it, a proper, clean way would be needed to check the origin of the 
> field. If for example an abstract model inheriting from a concrete model 
> adds some fields, you should be able to shadow the new fields, but not the 
> inherited fields. Making it internal would be clunky with the way cloning 
> and serializing currently works.
>
> I'm not too content with this anyway. As the default has to be `False` 
> (for an opt-in behaviour for abstract fields, rather than an opt-out 
> behaviour), this unnecessarily clutters migrations and other serialized 
> values with `locked=True` for concrete classes. However, it's not trivial 
> to consistently determine if a field is in any way inherited from a 
> concrete class. I will look into a clean method to determine this.
>
> I don't think we should remove it entirely, unless we provide another 
> mechanism to prevent field changes for some abstract fields. E.g. 
> django-polymorphic completely depends on the `polymorphic_ctype` field. 
> That might be a rare name to use, but it shows that some code does depends 
> on abstract fields. With the locking mechanism, we move the responsibility 
> of ensuring the right behaviour of that field to the package developer 
> rather than the website developer. I think a fail hard, fail fast approach 
> is better for these situations than going along with the possibility of 
> subtle or not-so-subtle bugs. 
>
> Op donderdag 12 februari 2015 17:59:58 UTC+1 schreef Loïc Bistuer:
>>
>> Hi Marten, 
>>
>> Sorry for the late response, it's been a busy week. Thanks for working on 
>> this, I'll try to have a look at your branch this weekend. 
>>
>> In the meantime I'll leave some comments below. 
>>
>> > On Feb 12, 2015, at 22:43, Marten Kenbeek <marte...@gmail.com> wrote: 
>> > 
>> > Current status: 
>> > • Models in general seem to be working. 
>> > • Trying to override a concrete (or explicitly locked) field 
>> with an abstract field will keep the concrete field, but not raise any 
>> errors. This occurs when inheriting from (AbstractBase, ConcreteBase). 
>> > • I might still change this to raise an error, it migth 
>> cause a few headaches when fields don't appear while they're first in the 
>> mro. 
>>
>> This should definitely be an error. We recently added checks so two 
>> concrete parents can't have clashing fields, this is similar. 
>>
>> > • Trying to override field.attname (e.g. foo_id) will fail 
>> checks unless you explicitly set foo = None. 
>> > • Trying to override an abstract field with a reverse relation 
>> will fail checks unless you explicitly set foo = None. 
>>
>> That's good. 
>>
>> > What is still needed: 
>> > • ? More extensive model tests. During development a few more 
>> bugs popped up. I've written tests for them, but there might be some more. 
>> > • Explicit migration tests. Existing migration tests pass, but 
>> don't include the new scenario's. 
>> > • Documentation. 
>> > My WIP branch can be found at 
>> https://github.com/knbk/django/compare/ticket_24305. 
>> > 
>>
>> > Op dinsdag 10 februari 2015 14:36:53 UTC+1 schreef Marten Kenbeek: 
>> > Hi Aron, 
>> > 
>> > With option 3, this would work: 
>> > 
>> > class Cousin(models.Model): 
>> > child = models.ForeignKey(Child, allow_override=True) 
>> > 
>> > but it would raise an error without `allow_override`. 
>> > 
>> > I think my question really is: should we treat reverse relations as 
>> first-class fields, or not? If we do, 1) would be the logical choice but 
>> can cause problems. 3) would deal with those problems in a way and still 
>> allow a relation field from another model to override a local field. If we 
>> don't, that kind of 

Re: Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

Hi Loïc,

Thanks for the response. I will change my code to generate errors in the 
case of abstract fields shadowing concrete fields. 

At the moment, the locking mechanism of fields is pretty much the core of 
this patch. It is explicitly set to `True` when fields are added to a 
concrete model, and it allows to easily check if a field can be changed. 
Without it, a proper, clean way would be needed to check the origin of the 
field. If for example an abstract model inheriting from a concrete model 
adds some fields, you should be able to shadow the new fields, but not the 
inherited fields. Making it internal would be clunky with the way cloning 
and serializing currently works.

I'm not too content with this anyway. As the default has to be `False` (for 
an opt-in behaviour for abstract fields, rather than an opt-out behaviour), 
this unnecessarily clutters migrations and other serialized values with 
`locked=True` for concrete classes. However, it's not trivial to 
consistently determine if a field is in any way inherited from a concrete 
class. I will look into a clean method to determine this.

I don't think we should remove it entirely, unless we provide another 
mechanism to prevent field changes for some abstract fields. E.g. 
django-polymorphic completely depends on the `polymorphic_ctype` field. 
That might be a rare name to use, but it shows that some code does depends 
on abstract fields. With the locking mechanism, we move the responsibility 
of ensuring the right behaviour of that field to the package developer 
rather than the website developer. I think a fail hard, fail fast approach 
is better for these situations than going along with the possibility of 
subtle or not-so-subtle bugs. 

Op donderdag 12 februari 2015 17:59:58 UTC+1 schreef Loïc Bistuer:
>
> Hi Marten, 
>
> Sorry for the late response, it's been a busy week. Thanks for working on 
> this, I'll try to have a look at your branch this weekend. 
>
> In the meantime I'll leave some comments below. 
>
> > On Feb 12, 2015, at 22:43, Marten Kenbeek <marte...@gmail.com 
> > wrote: 
> > 
> > Current status: 
> > • Models in general seem to be working. 
> > • Trying to override a concrete (or explicitly locked) field 
> with an abstract field will keep the concrete field, but not raise any 
> errors. This occurs when inheriting from (AbstractBase, ConcreteBase). 
> > • I might still change this to raise an error, it migth 
> cause a few headaches when fields don't appear while they're first in the 
> mro. 
>
> This should definitely be an error. We recently added checks so two 
> concrete parents can't have clashing fields, this is similar. 
>
> > • Trying to override field.attname (e.g. foo_id) will fail 
> checks unless you explicitly set foo = None. 
> > • Trying to override an abstract field with a reverse relation 
> will fail checks unless you explicitly set foo = None. 
>
> That's good. 
>
> > What is still needed: 
> > • ? More extensive model tests. During development a few more 
> bugs popped up. I've written tests for them, but there might be some more. 
> > • Explicit migration tests. Existing migration tests pass, but 
> don't include the new scenario's. 
> > • Documentation. 
> > My WIP branch can be found at 
> https://github.com/knbk/django/compare/ticket_24305. 
> > 
>
> > Op dinsdag 10 februari 2015 14:36:53 UTC+1 schreef Marten Kenbeek: 
> > Hi Aron, 
> > 
> > With option 3, this would work: 
> > 
> > class Cousin(models.Model): 
> > child = models.ForeignKey(Child, allow_override=True) 
> > 
> > but it would raise an error without `allow_override`. 
> > 
> > I think my question really is: should we treat reverse relations as 
> first-class fields, or not? If we do, 1) would be the logical choice but 
> can cause problems. 3) would deal with those problems in a way and still 
> allow a relation field from another model to override a local field. If we 
> don't, that kind of implies 2). Removing the field with `None` would 
> implicitly allow a reverse relation to take its place, but that reverse 
> relation in and of itself would not be allowed to override any abstract 
> fields. 
> > 
> > This is just an example. In real code this would more likely happen with 
> an explicit `related_name`. Renaming the related name and living with an 
> unused `cousin_set` attribute is the current solution, but with this 
> feature proposal it would be nice to also have a way to override the 
> inherited field and use it for a reverse relation instead. 
> > 
> > 
> > Op dinsdag 10 februari 2015 02:28:58 UTC+1 schreef Aron Podrigal: 
> > Why should t

Re: Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

Current status:

   - Models in general seem to be working.
   - Trying to override a concrete (or explicitly locked) field with an 
   abstract field will keep the concrete field, but not raise any errors. This 
   occurs when inheriting from (AbstractBase, ConcreteBase). 
  - I might still change this to raise an error, it migth cause a few 
  headaches when fields don't appear while they're first in the mro. 
   - Trying to override field.attname (e.g. foo_id) will fail checks unless 
   you explicitly set foo = None. 
   - Trying to override an abstract field with a reverse relation will fail 
   checks unless you explicitly set foo = None.

What is still needed:

   - ? More extensive model tests. During development a few more bugs 
   popped up. I've written tests for them, but there might be some more.
   - Explicit migration tests. Existing migration tests pass, but don't 
   include the new scenario's.
   - Documentation. 

My WIP branch can be found at 
https://github.com/knbk/django/compare/ticket_24305.

Op dinsdag 10 februari 2015 14:36:53 UTC+1 schreef Marten Kenbeek:
>
> Hi Aron,
>
> With option 3, this would work:
>
> class Cousin(models.Model):
> child = models.ForeignKey(Child, allow_override=True)
>
> but it would raise an error without `allow_override`. 
>
> I think my question really is: should we treat reverse relations as 
> first-class fields, or not? If we do, 1) would be the logical choice but 
> can cause problems. 3) would deal with those problems in a way and still 
> allow a relation field from another model to override a local field. If we 
> don't, that kind of implies 2). Removing the field with `None` would 
> implicitly allow a reverse relation to take its place, but that reverse 
> relation in and of itself would not be allowed to override any abstract 
> fields. 
>
> This is just an example. In real code this would more likely happen with 
> an explicit `related_name`. Renaming the related name and living with an 
> unused `cousin_set` attribute is the current solution, but with this 
> feature proposal it would be nice to also have a way to override the 
> inherited field and use it for a reverse relation instead. 
>
>
> Op dinsdag 10 februari 2015 02:28:58 UTC+1 schreef Aron Podrigal:
>>
>> Why should this be treated differently than the general behavior when 
>> realted_names clash when you have more than one foreign key to the same 
>> model? So as one would normally do
>> set the related_name explicitly to some other value.
>> setting the field to None is just the way of removing a field and has 
>> nothing more special  related to the auto created reverse field descriptor.
>> about option 3 I didn't quite understand. can you explain a bit more?
>>
>> On Monday, February 9, 2015 at 4:25:22 PM UTC-5, Marten Kenbeek wrote:
>>>
>>> I'd like some additional opinions on the following:
>>>
>>> Say you have the following classes:
>>>
>>> class Parent(models.Model):
>>> cousin_set = models.CharField(max_length=100)
>>>
>>> class Meta:
>>> abstract = True
>>>
>>> class Child(Parent):
>>> pass
>>>
>>> class Cousin(models.Model):
>>> child = models.ForeignKey(Child)
>>>
>>> Obviously, the `cousin_set` field inherited from `Parent` clashes with 
>>> the reverse relation from `Cousin`. 
>>>
>>> I can see the following options:
>>> 1) Allow the reverse object descriptor to overwrite the field. 
>>> 2) Only allow the reverse object descriptor to overwrite the field if it 
>>> is explicitly set to None on the target model.
>>> 3) Only allow the reverse object descriptor to overwrite the field if 
>>> the foreignkey/m2m/o2o field itself has a flag set to explicitly override.
>>>
>>> 1) is consistent with the behaviour of local fields, but I think it will 
>>> be problematic if *other *models can silently overwrite a field. 3) 
>>> would still allow other models to affect local fields, but at least it has 
>>> a fail-safe that prevents you from accidentally overriding fields. 2) would 
>>> only allow the inheriting model itself to change which fields it inherits. 
>>>
>>> Problems caused by option 1) would be hard to debug when you don't know 
>>> which code overrides your field, so I wouldn't do that. I think 2) would be 
>>> the cleanest and most consistent way. Only local fields would override 
>>> parent fields, but the sentinel value `None` would remove the field and 
>>> free the name for reverse relations. I can also see the advantage of 3) 
>>> over 2) w

Re: Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

Hi Aron,

With option 3, this would work:

class Cousin(models.Model):
child = models.ForeignKey(Child, allow_override=True)

but it would raise an error without `allow_override`. 

I think my question really is: should we treat reverse relations as 
first-class fields, or not? If we do, 1) would be the logical choice but 
can cause problems. 3) would deal with those problems in a way and still 
allow a relation field from another model to override a local field. If we 
don't, that kind of implies 2). Removing the field with `None` would 
implicitly allow a reverse relation to take its place, but that reverse 
relation in and of itself would not be allowed to override any abstract 
fields. 

This is just an example. In real code this would more likely happen with an 
explicit `related_name`. Renaming the related name and living with an 
unused `cousin_set` attribute is the current solution, but with this 
feature proposal it would be nice to also have a way to override the 
inherited field and use it for a reverse relation instead. 


Op dinsdag 10 februari 2015 02:28:58 UTC+1 schreef Aron Podrigal:
>
> Why should this be treated differently than the general behavior when 
> realted_names clash when you have more than one foreign key to the same 
> model? So as one would normally do
> set the related_name explicitly to some other value.
> setting the field to None is just the way of removing a field and has 
> nothing more special  related to the auto created reverse field descriptor.
> about option 3 I didn't quite understand. can you explain a bit more?
>
> On Monday, February 9, 2015 at 4:25:22 PM UTC-5, Marten Kenbeek wrote:
>>
>> I'd like some additional opinions on the following:
>>
>> Say you have the following classes:
>>
>> class Parent(models.Model):
>> cousin_set = models.CharField(max_length=100)
>>
>> class Meta:
>> abstract = True
>>
>> class Child(Parent):
>> pass
>>
>> class Cousin(models.Model):
>> child = models.ForeignKey(Child)
>>
>> Obviously, the `cousin_set` field inherited from `Parent` clashes with 
>> the reverse relation from `Cousin`. 
>>
>> I can see the following options:
>> 1) Allow the reverse object descriptor to overwrite the field. 
>> 2) Only allow the reverse object descriptor to overwrite the field if it 
>> is explicitly set to None on the target model.
>> 3) Only allow the reverse object descriptor to overwrite the field if the 
>> foreignkey/m2m/o2o field itself has a flag set to explicitly override.
>>
>> 1) is consistent with the behaviour of local fields, but I think it will 
>> be problematic if *other *models can silently overwrite a field. 3) 
>> would still allow other models to affect local fields, but at least it has 
>> a fail-safe that prevents you from accidentally overriding fields. 2) would 
>> only allow the inheriting model itself to change which fields it inherits. 
>>
>> Problems caused by option 1) would be hard to debug when you don't know 
>> which code overrides your field, so I wouldn't do that. I think 2) would be 
>> the cleanest and most consistent way. Only local fields would override 
>> parent fields, but the sentinel value `None` would remove the field and 
>> free the name for reverse relations. I can also see the advantage of 3) 
>> over 2) when you don't have access to the model on the other side. However, 
>> I don't know enough about foreign key internals to know if 3) is at all 
>> feasible. What happens e.g. when only the target is loaded in a migration? 
>> Would it pick up that the remote foreign key overrides a local field? As 
>> adding reverse relations is a lazy, or at least delayed operation afaik, 
>> would it still be save to rely on that to override fields?
>>
>> I believe my current plans for the patch would automatically create 
>> situation 2 without any extra work. The field would no longer exist on the 
>> child class when the reverse relation is added. Option 3) would require an 
>> additional patch to the code that adds the reverse relationship, but it 
>> allows for some extra options.
>>
>> Any input? Additional options are also welcome. 
>>
>> Op zondag 8 februari 2015 21:09:41 UTC+1 schreef Marten Kenbeek:
>>>
>>> The general reaction seems to be a yes if we can work out the kinks, so 
>>> I went ahead and created a ticket: 
>>> https://code.djangoproject.com/ticket/24305
>>>
>>> @Aron That's a good question. One option would be to lock certain 
>>> fields, so that they can't be changed if they are an integral part of the 
>>> model. That would be a simple 

Re: Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

I'd like some additional opinions on the following:

Say you have the following classes:

class Parent(models.Model):
cousin_set = models.CharField(max_length=100)

class Meta:
abstract = True

class Child(Parent):
pass

class Cousin(models.Model):
child = models.ForeignKey(Child)

Obviously, the `cousin_set` field inherited from `Parent` clashes with the 
reverse relation from `Cousin`. 

I can see the following options:
1) Allow the reverse object descriptor to overwrite the field. 
2) Only allow the reverse object descriptor to overwrite the field if it is 
explicitly set to None on the target model.
3) Only allow the reverse object descriptor to overwrite the field if the 
foreignkey/m2m/o2o field itself has a flag set to explicitly override.

1) is consistent with the behaviour of local fields, but I think it will be 
problematic if *other *models can silently overwrite a field. 3) would 
still allow other models to affect local fields, but at least it has a 
fail-safe that prevents you from accidentally overriding fields. 2) would 
only allow the inheriting model itself to change which fields it inherits. 

Problems caused by option 1) would be hard to debug when you don't know 
which code overrides your field, so I wouldn't do that. I think 2) would be 
the cleanest and most consistent way. Only local fields would override 
parent fields, but the sentinel value `None` would remove the field and 
free the name for reverse relations. I can also see the advantage of 3) 
over 2) when you don't have access to the model on the other side. However, 
I don't know enough about foreign key internals to know if 3) is at all 
feasible. What happens e.g. when only the target is loaded in a migration? 
Would it pick up that the remote foreign key overrides a local field? As 
adding reverse relations is a lazy, or at least delayed operation afaik, 
would it still be save to rely on that to override fields?

I believe my current plans for the patch would automatically create 
situation 2 without any extra work. The field would no longer exist on the 
child class when the reverse relation is added. Option 3) would require an 
additional patch to the code that adds the reverse relationship, but it 
allows for some extra options.

Any input? Additional options are also welcome. 

Op zondag 8 februari 2015 21:09:41 UTC+1 schreef Marten Kenbeek:
>
> The general reaction seems to be a yes if we can work out the kinks, so I 
> went ahead and created a ticket: 
> https://code.djangoproject.com/ticket/24305
>
> @Aron That's a good question. One option would be to lock certain fields, 
> so that they can't be changed if they are an integral part of the model. 
> That would be a simple solution, but that won't help for existing code that 
> doesn't lock the fields. It won't break existing code, but it won't protect 
> for errors either. The opt-in version (i.e. an 'unlock' attribute) would 
> lock many fields which would otherwise be completely safe to overwrite. 
>
> Another option would be more elaborate "requirements" for a manager or 
> some methods, i.e. allow the manager to specify the necessary class of a 
> certain field or a minimum length. If the modeldoesn't meet the 
> requirements, the manager or some of the methods will not be inherited. 
> While it allows for more control, this option would greatly increase the 
> complexity of the patch and requires more from the developers of custom 
> managers. It can also cause issues when the requirements aren't up-to-date 
> with the manager's methods. 
>
> We could also say that it is the users responsibility and don't provide 
> special protection, in line with the fields themselves, but I guess that 
> this would generally be more problematic for custom managers. It can also 
> cause silent bugs when the manager's methods don't work as intended but 
> won't raise an exception either, which is not a good idea imho. 
>
> I think the locking approach would be the easiest and most pragmatic 
> method. I think it's still - in part - the users responsibility to confirm 
> that a field can be overridden. The Django documentation could, where 
> applicable, document the requirements on fields that can be overridden, 
> i.e. that an AbstractUser's username must be a CharField (which isn't 
> necessarily true, just an example). 
>
> @Loïc The bases are indeed traversed in the reversed order. 
>
> Op zondag 8 februari 2015 08:19:57 UTC+1 schreef Loïc Bistuer:
>>
>> That's what we've done in Django 1.7 for Form/ModelForm (#19617, #8620), 
>> and I completely agree that it should be possible to shadow fields from 
>> abstract models. The docs even suggest that this may be possible in the 
>> future; quoting the relevant section: "this [shadowing] is not permitted 
>> for attributes that are F

Re: Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

The general reaction seems to be a yes if we can work out the kinks, so I 
went ahead and created a ticket: https://code.djangoproject.com/ticket/24305

@Aron That's a good question. One option would be to lock certain fields, 
so that they can't be changed if they are an integral part of the model. 
That would be a simple solution, but that won't help for existing code that 
doesn't lock the fields. It won't break existing code, but it won't protect 
for errors either. The opt-in version (i.e. an 'unlock' attribute) would 
lock many fields which would otherwise be completely safe to overwrite. 

Another option would be more elaborate "requirements" for a manager or some 
methods, i.e. allow the manager to specify the necessary class of a certain 
field or a minimum length. If the modeldoesn't meet the requirements, the 
manager or some of the methods will not be inherited. While it allows for 
more control, this option would greatly increase the complexity of the 
patch and requires more from the developers of custom managers. It can also 
cause issues when the requirements aren't up-to-date with the manager's 
methods. 

We could also say that it is the users responsibility and don't provide 
special protection, in line with the fields themselves, but I guess that 
this would generally be more problematic for custom managers. It can also 
cause silent bugs when the manager's methods don't work as intended but 
won't raise an exception either, which is not a good idea imho. 

I think the locking approach would be the easiest and most pragmatic 
method. I think it's still - in part - the users responsibility to confirm 
that a field can be overridden. The Django documentation could, where 
applicable, document the requirements on fields that can be overridden, 
i.e. that an AbstractUser's username must be a CharField (which isn't 
necessarily true, just an example). 

@Loïc The bases are indeed traversed in the reversed order. 

Op zondag 8 februari 2015 08:19:57 UTC+1 schreef Loïc Bistuer:
>
> That's what we've done in Django 1.7 for Form/ModelForm (#19617, #8620), 
> and I completely agree that it should be possible to shadow fields from 
> abstract models. The docs even suggest that this may be possible in the 
> future; quoting the relevant section: "this [shadowing] is not permitted 
> for attributes that are Field instances (at least, not at the moment)." 
>
> For consistency with forms - and because we've put a great deal of 
> thinking into it - the implementation should be: you can shadow a field 
> with another field, or you can remove a field using None as a sentinel 
> value (see #22510 for more details). 
>
> I believe we have a safety net in the form of migrations, if you 
> accidentally shadow a field, migrations will pick it up, so it's unlikely 
> to go unnoticed. 
>
> I'd be quite happy to shepherd this patch. 
>
> Unit tests should cover those scenarios: 
>
> - An abstract model redefines or removes some fields from a parent 
> abstract model. 
> - A concrete model redefines or removes some fields from a parent abstract 
> model. 
> - Ensure that the standard MRO resolution applies when you do 
> Concrete(AbstractA, AbstractB) with AbstractA redefining a field from 
> AbstractB. 
>
> The last case may be tricky because if I remember well ModelBase iterates 
> through the MRO in the wrong direction (i.e. bases should be iterated over 
> in reverse). 
>
> We also need some tests to show how migrations behave. 
>
> -- 
> Loïc 
>
> > On Feb 7, 2015, at 09:33, Marten Kenbeek <marte...@gmail.com 
> > wrote: 
> > 
> > Hi Russ, 
> > 
> > I can see your point on accidentally overriding fields, though I'm not 
> sure I agree. In any other case, such as normal attributes and methods, 
> there is no safety net for overriding attributes either. Any model that 
> would be affected by this change would also raise an error on import 
> without the patch, so existing functional code won't be affected. On the 
> other hand, a new parameter for the field wouldn't be that much of a hassle 
> to implement or work with. I'd be interested to hear what others think 
> about this. 
> > 
> > There are more than a few questions on stack overflow that expect this 
> behaviour, even if the docs specifically mention that it won't work. If 
> users intuitively try to override fields in this manner, I think it would 
> be reasonable to allow this without any explicit arguments. 
> > 
> > We can always restrict what you can override, at least as the default 
> behaviour, e.g. so that you can only use subclasses of the original field. 
> That would make code that depends on the abstract field less prone to bugs, 
> though subtle bugs can still happen if you e.g. override the max length 

Re: Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

Hi Russ,

I can see your point on accidentally overriding fields, though I'm not sure 
I agree. In any other case, such as normal attributes and methods, there is 
no safety net for overriding attributes either. Any model that would be 
affected by this change would also raise an error on import without the 
patch, so existing functional code won't be affected. On the other hand, a 
new parameter for the field wouldn't be that much of a hassle to implement 
or work with. I'd be interested to hear what others think about this.

There are more than a few questions 
<http://stackoverflow.com/search?q=%5Bdjango%5D%5Bdjango-models%5D+override+field+is%3Aquestion>
 
on stack overflow that expect this behaviour, even if the docs specifically 
mention that it won't work. If users intuitively try to override fields in 
this manner, I think it would be reasonable to allow this without any 
explicit arguments.

We can always restrict what you can override, at least as the default 
behaviour, e.g. so that you can only use subclasses of the original field. 
That would make code that depends on the abstract field less prone to bugs, 
though subtle bugs can still happen if you e.g. override the max length of 
a field. 

This was indeed just a proof-of-concept. That remark was meant more like 
"it doesn't appear to break everything". 

Marten

Op vrijdag 6 februari 2015 23:48:55 UTC+1 schreef Marten Kenbeek:
>
> Hey,
>
> While fiddling with django I noticed it would be trivial to add support 
> for shadowing fields from abstract base models. As abstract fields don't 
> exist in the database, you simply have to remove the check that reports 
> name clashes for abstract models, and no longer override the field. 
>
> This would allow you to both override fields from third-party abstract 
> models (e.g. change the username length of AbstractUser), and to override 
> fields of common abstract models on a per-model basis. Previously you'd 
> have to copy the original abstract model just to change a single field, or 
> alter the field after the model definition. The first approach violates the 
> DRY principle, the second approach can introduce subtle bugs if the field 
> is somehow used before you changed it (rare, but it can happen). 
>
> This patch 
> <https://github.com/knbk/django/commit/e8347457bccaeab11f9624b01c57824f4511da6c>
>  adds 
> the feature. It seems to work correctly when using the models and 
> creating/applying migrations, and passes the complete test suite. 
>
> What do you guys think about this feature?
>

-- 
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/072149e1-c794-446d-957b-f5fc5df87096%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Feature proposal: Allow shadowing of abstract fields

[Marten Kenbeek]

Hey,

While fiddling with django I noticed it would be trivial to add support for 
shadowing fields from abstract base models. As abstract fields don't exist 
in the database, you simply have to remove the check that reports name 
clashes for abstract models, and no longer override the field. 

This would allow you to both override fields from third-party abstract 
models (e.g. change the username length of AbstractUser), and to override 
fields of common abstract models on a per-model basis. Previously you'd 
have to copy the original abstract model just to change a single field, or 
alter the field after the model definition. The first approach violates the 
DRY principle, the second approach can introduce subtle bugs if the field 
is somehow used before you changed it (rare, but it can happen). 

This patch 

 adds 
the feature. It seems to work correctly when using the models and 
creating/applying migrations, and passes the complete test suite. 

What do you guys think about this feature?

-- 
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/f846521f-8a16-41c7-9998-2bea7c93b3dd%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.