On Sun, Mar 28, 2010 at 6:59 AM, Andrew Godwin <[email protected]> wrote: > On 27/03/10 17:08, Russell Keith-Magee wrote: >> >> There are 5 options I can see. >> >> Option 1: Do nothing. #13087 describes a use case we don't want to >> support, so we ignore it. >> > > I think it should be supported; it seems like a reasonable suggestion, and I > can see reasons for implementing things that maintain M2M relationships in > this way. > >> Option 2: We add a "cleared" signal that occurs after the clear >> actually occurs. This solves the use case for #13087, but only adds 1 >> signal. >> >> Option 3: We modify the existing signals so we have a pre-post pair >> for every signal. This maintains the analog with pre/post save, and >> gives the most control. For example, on Alex Gaynor has suggested to >> me that some people might want to use a pre-add signal rather than a >> post-add signal for cache invalidation since there is a marginally >> lower chance of getting a race condition. However, signals aren't free >> -- an unattached signal is roughly equivalent to the overhead of a >> function call. >> >> Option 4: (1), but also move add and remove to be *pre* signals, to >> alleviate Alex's concern from (3) >> >> Option 5: (2), but also move add and remove to be *pre* signals, to >> alleviate Alex's concern from (3) >> > > If the overhead isn't too great I'd personally prefer (3), as it's the > cleanest solution, and the one that seems most obvious (consistency makes me > feel all warm and fuzzy inside). However, since clear() isn't the same as > the other two, (2) also seems reasonable enough to me. > > The only use-cases I can think of where you'd have to use a *pre* signal for > add or remove would be to stop them happening (i.e. enforce constraints at > the model level), and this seems like the Wrong Way to do things. On the > contrary, there are more reasons to have them there *post* action, as you > can then safely modify the relationship again/do some raw database queries > on the data/use the relationship to do traversal or iteration over the > contents (although one might argue that's also a case for having a > pre-remove).
Cache invalidation is a reasonably compelling case for pre-signals; if you invalidate a cache on the post-signal, there is a small window between having modified the m2m and the cache being flushed. In that window, any operation hitting the cache will see the m2m relations still existing, but any operation that actually hits the database will disagree. It's a really small edge case, but that's the class of Heisenbug that is a pain to find in the wild. However, as you note, any raw SQL activity will probably need to happen in the post signal, so that's probably enough of a reason to include pre and post signals. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
