Re: Clearing prefetch related on add(), change(), remove()

2016-06-08 Thread Josh Smeaton 
Usually, yes. I'm not sure refresh_from_db works on related managers 
though. I ran into a similar issue writing unit test fixtures just last 
week and refresh_from_db didn't fix the problem.



On Thursday, 9 June 2016 02:46:25 UTC+10, bliy...@rentlytics.com wrote:
>
> To be clear, I think the way to force the refresh of an orm object is to 
> use the `refresh_from_db` method.  Is that functionally equivalent?
>
> -Ben
>
> On Tuesday, June 7, 2016 at 6:12:30 AM UTC-7, Marc Tamlyn wrote:
>>
>> I may be "too close" to knowing the implementation of this feature to be 
>> able to comment on whether the behaviour is surprising to most people, but 
>> it doesn't surprise me. It's certainly analogous to that when you do 
>> `MyModel.objects.create()` it doesn't change an already executed queryset. 
>> There's a question of where you draw the line as well - what about 
>> `related_set.update()`?
>>
>> I think it's worth documenting the behaviour, also noting that you can 
>> "force" the execution of a new queryset by chaining another .all().
>>
>> On 7 June 2016 at 13:26, Yoong Kang Lim  wrote:
>>
>>> Hi all, I'd like to bring up ticket #26706: 
>>> https://code.djangoproject.com/ticket/26706
>>>
>>> Related managers have methods such as add(), change() and remove() that 
>>> change database objects. When a prefetch_related is done prior to calling 
>>> these methods, it does not clear the cache. When the related field is 
>>> accessed, it returns the cached result instead of the updated result. A 
>>> couple of tickets have been opened, as this does seem to be surprising 
>>> behaviour.
>>>
>>> I was working on a patch to address this, but Tim brought up some 
>>> concerns about backward compatibility regarding the change and directed me 
>>> here to get some community consensus. The change I'm proposing will clear 
>>> the cache (for the prefetched field) when any of the methods are called. If 
>>> we introduce this, it will be a backwards-incompatible change, so I'd just 
>>> like to get some opinions on what the best way forward would be. Obviously 
>>> in either case the behaviour should be documented. 
>>>
>>> Also a thought just occurred to me -- if we don't put this change in, 
>>> could we, as an alternative solution, extend the API to let the user decide 
>>> what to do with the cache? Maybe something like 
>>> clear_prefetched_field(related_field_name) on the manager so that at least 
>>> the user has a choice instead of running the query (although the trouble 
>>> they would need to go through would be similar, IMO).
>>>
>>> -- 
>>> 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/eabd9567-53e3-413b-9b30-dbcfbf9c2634%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/86877070-614e-4104-b92c-a728ae1f0b49%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Clearing prefetch related on add(), change(), remove()

2016-06-08 Thread bliyanage 
To be clear, I think the way to force the refresh of an orm object is to 
use the `refresh_from_db` method.  Is that functionally equivalent?

-Ben

On Tuesday, June 7, 2016 at 6:12:30 AM UTC-7, Marc Tamlyn wrote:
>
> I may be "too close" to knowing the implementation of this feature to be 
> able to comment on whether the behaviour is surprising to most people, but 
> it doesn't surprise me. It's certainly analogous to that when you do 
> `MyModel.objects.create()` it doesn't change an already executed queryset. 
> There's a question of where you draw the line as well - what about 
> `related_set.update()`?
>
> I think it's worth documenting the behaviour, also noting that you can 
> "force" the execution of a new queryset by chaining another .all().
>
> On 7 June 2016 at 13:26, Yoong Kang Lim  > wrote:
>
>> Hi all, I'd like to bring up ticket #26706: 
>> https://code.djangoproject.com/ticket/26706
>>
>> Related managers have methods such as add(), change() and remove() that 
>> change database objects. When a prefetch_related is done prior to calling 
>> these methods, it does not clear the cache. When the related field is 
>> accessed, it returns the cached result instead of the updated result. A 
>> couple of tickets have been opened, as this does seem to be surprising 
>> behaviour.
>>
>> I was working on a patch to address this, but Tim brought up some 
>> concerns about backward compatibility regarding the change and directed me 
>> here to get some community consensus. The change I'm proposing will clear 
>> the cache (for the prefetched field) when any of the methods are called. If 
>> we introduce this, it will be a backwards-incompatible change, so I'd just 
>> like to get some opinions on what the best way forward would be. Obviously 
>> in either case the behaviour should be documented. 
>>
>> Also a thought just occurred to me -- if we don't put this change in, 
>> could we, as an alternative solution, extend the API to let the user decide 
>> what to do with the cache? Maybe something like 
>> clear_prefetched_field(related_field_name) on the manager so that at least 
>> the user has a choice instead of running the query (although the trouble 
>> they would need to go through would be similar, IMO).
>>
>> -- 
>> 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/eabd9567-53e3-413b-9b30-dbcfbf9c2634%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/21f57884-ba0a-40e4-9a81-28645d059f03%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Clearing prefetch related on add(), change(), remove()

2016-06-08 Thread bliyanage 
I am for the clearing of the cache--that behavior seems weird.  If you 
didn't want the cache to clear you would probably be using a different orm 
object to do your query.

Just to be clear, after clearing the cache, any future requests against 
that data will be lazily evaluated right?

-Ben

On Tuesday, June 7, 2016 at 5:26:42 AM UTC-7, Yoong Kang Lim wrote:
>
> Hi all, I'd like to bring up ticket #26706: 
> https://code.djangoproject.com/ticket/26706
>
> Related managers have methods such as add(), change() and remove() that 
> change database objects. When a prefetch_related is done prior to calling 
> these methods, it does not clear the cache. When the related field is 
> accessed, it returns the cached result instead of the updated result. A 
> couple of tickets have been opened, as this does seem to be surprising 
> behaviour.
>
> I was working on a patch to address this, but Tim brought up some concerns 
> about backward compatibility regarding the change and directed me here to 
> get some community consensus. The change I'm proposing will clear the cache 
> (for the prefetched field) when any of the methods are called. If we 
> introduce this, it will be a backwards-incompatible change, so I'd just 
> like to get some opinions on what the best way forward would be. Obviously 
> in either case the behaviour should be documented. 
>
> Also a thought just occurred to me -- if we don't put this change in, 
> could we, as an alternative solution, extend the API to let the user decide 
> what to do with the cache? Maybe something like 
> clear_prefetched_field(related_field_name) on the manager so that at least 
> the user has a choice instead of running the query (although the trouble 
> they would need to go through would be similar, IMO).
>

-- 
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/ad5c2d3f-e00c-42f6-b519-4d294a6c0c78%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Clearing prefetch related on add(), change(), remove()

2016-06-08 Thread Marc Tamlyn 
I didn't know queryset.update did clear the internal cache. In that case
it's pretty reasonable. I think it should only clear a .all() cache though,
not any prefetched set.

On 7 June 2016 at 22:38, Florian Apolloner  wrote:

> Same feeling as Carl here. I was probably the first to get asked whether
> or not this was a bug or not on IRC and my initial thought was also "wtf,
> that is clearly a bug" -- hence I asked Yoong Kang Lim to open a ticket.
>
> Cheers,
> Florian
>
>
> On Tuesday, June 7, 2016 at 7:47:29 PM UTC+2, Carl Meyer wrote:
>>
>> On 06/07/2016 06:11 AM, Marc Tamlyn wrote:
>> > I may be "too close" to knowing the implementation of this feature to
>> be
>> > able to comment on whether the behaviour is surprising to most people,
>> > but it doesn't surprise me. It's certainly analogous to that when you
>> do
>> > `MyModel.objects.create()` it doesn't change an already executed
>> > queryset. There's a question of where you draw the line as well - what
>> > about `related_set.update()`?
>> >
>> > I think it's worth documenting the behaviour, also noting that you can
>> > "force" the execution of a new queryset by chaining another .all().
>>
>> Hmm, I have the opposite instinct. I don't find it analogous to the case
>> of some unrelated queryset object failing to update its internal cache
>> when the database changes. In this case we have a related-manager with
>> an internal cache, and we make changes _via that same manager_. I find
>> it quite surprising that the manager doesn't automatically clear its
>> cache in that case.
>>
>> A much stronger precedent, I think, is the fact that Queryset.update()
>> does clear the queryset's internal result cache. In light of that, I
>> think the current behavior with prefetched-data on a related manager is
>> a bug that should be fixed (though it certainly should be mentioned in
>> the release notes).
>>
>> Carl
>>
>> --
> 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/61cce517-3117-42ec-aceb-de8bb3a7b7cc%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/CAMwjO1H0%3D%2BKefZPwBCkBx5rX722rjp84n5Re2mSLYSAVtq74yA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Clearing prefetch related on add(), change(), remove()

2016-06-07 Thread Florian Apolloner 
Same feeling as Carl here. I was probably the first to get asked whether or 
not this was a bug or not on IRC and my initial thought was also "wtf, that 
is clearly a bug" -- hence I asked Yoong Kang Lim to open a ticket.

Cheers,
Florian

On Tuesday, June 7, 2016 at 7:47:29 PM UTC+2, Carl Meyer wrote:
>
> On 06/07/2016 06:11 AM, Marc Tamlyn wrote: 
> > I may be "too close" to knowing the implementation of this feature to be 
> > able to comment on whether the behaviour is surprising to most people, 
> > but it doesn't surprise me. It's certainly analogous to that when you do 
> > `MyModel.objects.create()` it doesn't change an already executed 
> > queryset. There's a question of where you draw the line as well - what 
> > about `related_set.update()`? 
> > 
> > I think it's worth documenting the behaviour, also noting that you can 
> > "force" the execution of a new queryset by chaining another .all(). 
>
> Hmm, I have the opposite instinct. I don't find it analogous to the case 
> of some unrelated queryset object failing to update its internal cache 
> when the database changes. In this case we have a related-manager with 
> an internal cache, and we make changes _via that same manager_. I find 
> it quite surprising that the manager doesn't automatically clear its 
> cache in that case. 
>
> A much stronger precedent, I think, is the fact that Queryset.update() 
> does clear the queryset's internal result cache. In light of that, I 
> think the current behavior with prefetched-data on a related manager is 
> a bug that should be fixed (though it certainly should be mentioned in 
> the release notes). 
>
> Carl 
>
>

-- 
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/61cce517-3117-42ec-aceb-de8bb3a7b7cc%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Clearing prefetch related on add(), change(), remove()

2016-06-07 Thread Carl Meyer 
On 06/07/2016 06:11 AM, Marc Tamlyn wrote:
> I may be "too close" to knowing the implementation of this feature to be
> able to comment on whether the behaviour is surprising to most people,
> but it doesn't surprise me. It's certainly analogous to that when you do
> `MyModel.objects.create()` it doesn't change an already executed
> queryset. There's a question of where you draw the line as well - what
> about `related_set.update()`?
> 
> I think it's worth documenting the behaviour, also noting that you can
> "force" the execution of a new queryset by chaining another .all().

Hmm, I have the opposite instinct. I don't find it analogous to the case
of some unrelated queryset object failing to update its internal cache
when the database changes. In this case we have a related-manager with
an internal cache, and we make changes _via that same manager_. I find
it quite surprising that the manager doesn't automatically clear its
cache in that case.

A much stronger precedent, I think, is the fact that Queryset.update()
does clear the queryset's internal result cache. In light of that, I
think the current behavior with prefetched-data on a related manager is
a bug that should be fixed (though it certainly should be mentioned in
the release notes).

Carl

-- 
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/575708A1.902%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature

Re: Clearing prefetch related on add(), change(), remove()

2016-06-07 Thread Marc Tamlyn 
I may be "too close" to knowing the implementation of this feature to be
able to comment on whether the behaviour is surprising to most people, but
it doesn't surprise me. It's certainly analogous to that when you do
`MyModel.objects.create()` it doesn't change an already executed queryset.
There's a question of where you draw the line as well - what about
`related_set.update()`?

I think it's worth documenting the behaviour, also noting that you can
"force" the execution of a new queryset by chaining another .all().

On 7 June 2016 at 13:26, Yoong Kang Lim  wrote:

> Hi all, I'd like to bring up ticket #26706:
> https://code.djangoproject.com/ticket/26706
>
> Related managers have methods such as add(), change() and remove() that
> change database objects. When a prefetch_related is done prior to calling
> these methods, it does not clear the cache. When the related field is
> accessed, it returns the cached result instead of the updated result. A
> couple of tickets have been opened, as this does seem to be surprising
> behaviour.
>
> I was working on a patch to address this, but Tim brought up some concerns
> about backward compatibility regarding the change and directed me here to
> get some community consensus. The change I'm proposing will clear the cache
> (for the prefetched field) when any of the methods are called. If we
> introduce this, it will be a backwards-incompatible change, so I'd just
> like to get some opinions on what the best way forward would be. Obviously
> in either case the behaviour should be documented.
>
> Also a thought just occurred to me -- if we don't put this change in,
> could we, as an alternative solution, extend the API to let the user decide
> what to do with the cache? Maybe something like
> clear_prefetched_field(related_field_name) on the manager so that at least
> the user has a choice instead of running the query (although the trouble
> they would need to go through would be similar, IMO).
>
> --
> 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/eabd9567-53e3-413b-9b30-dbcfbf9c2634%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/CAMwjO1EkD_gfr6rGrFe%3DGY5YRXCs5qYoKYmw8zST94GLJOHECA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Clearing prefetch related on add(), change(), remove()

2016-06-07 Thread Yoong Kang Lim 
Hi all, I'd like to bring up ticket 
#26706: https://code.djangoproject.com/ticket/26706

Related managers have methods such as add(), change() and remove() that 
change database objects. When a prefetch_related is done prior to calling 
these methods, it does not clear the cache. When the related field is 
accessed, it returns the cached result instead of the updated result. A 
couple of tickets have been opened, as this does seem to be surprising 
behaviour.

I was working on a patch to address this, but Tim brought up some concerns 
about backward compatibility regarding the change and directed me here to 
get some community consensus. The change I'm proposing will clear the cache 
(for the prefetched field) when any of the methods are called. If we 
introduce this, it will be a backwards-incompatible change, so I'd just 
like to get some opinions on what the best way forward would be. Obviously 
in either case the behaviour should be documented. 

Also a thought just occurred to me -- if we don't put this change in, could 
we, as an alternative solution, extend the API to let the user decide what 
to do with the cache? Maybe something like 
clear_prefetched_field(related_field_name) on the manager so that at least 
the user has a choice instead of running the query (although the trouble 
they would need to go through would be similar, IMO).

-- 
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/eabd9567-53e3-413b-9b30-dbcfbf9c2634%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.