Signals for ManyToMany relations question

[Ryan K]

This message is in regards to the patch on Ticket 5390 (http://
code.djangoproject.com/ticket/5390). I know that the period for
changes is closed but why shouldn't the current API just work. It
seems like a special case is being treated very differently but since
there relation is already implied it seems natural for Model.ManyToMany
(foo) to access an updated object instead of writing special case
handlers. The downside might be that too much happens in the handler
but I think model relations should have to comply to its related
model's signals. Perhaps this is just nitpicking and it's probably too
late.

Cheers,
Ryan

[Russell Keith-Magee]

On Fri, Dec 4, 2009 at 10:23 AM, Ryan K <mob...@ryankaskel.com> wrote:
> This message is in regards to the patch on Ticket 5390 (http://
> code.djangoproject.com/ticket/5390). I know that the period for
> changes is closed

The period for changes isn't closed at all - we're in the feature
development phase until Dec 22 [1]. Any feature with a working
implementation is a candidate for inclusion up to that date, as long
as you can convince a core developer to commit the code.

As a guideline, #5390 was almost included in Django 1.1 [2], but we
ran out of time. It wasn't explicitly included in the v1.2 voting, . I
was hoping to look at this ticket myself before the feature deadline,
but I've been tied down with other work. With luck, I will get a
chance to look at this over the next week or so.

[1] http://code.djangoproject.com/wiki/Version1.2Roadmap
[2] http://code.djangoproject.com/wiki/Version1.1Features

> but why shouldn't the current API just work. It
> seems like a special case is being treated very differently but since
> there relation is already implied it seems natural for Model.ManyToMany
> (foo) to access an updated object instead of writing special case
> handlers. The downside might be that too much happens in the handler
> but I think model relations should have to comply to its related
> model's signals. Perhaps this is just nitpicking and it's probably too
> late.

I'm completely confused over what you're referring to here. Are you
referring to a particular patch, or current trunk? Are you proposing
an alternate patch?

Yours,
Russ Magee %-)

[Ryan K]

Sorry for the confusion but I sent the original link from my mobile
handset which isn't great for copying/pasting/etc.

In another post, we were discussing the issue of ManyToMany updates
[1] and how the signal handler for a model X that has a
ManyToManyField to Y will _not_ have an updated Y. It's not
necessarily a "bug" but its feature that's lacking and it's been in
discussion for many years. Here is the use case from the ticket:

For example, I want to send en e-mail when an Account is created to
Account.email. An account has a ManyToManyField to a Location model
and I want to for whatever reason include the location(s) in the e-
mail. If the location was updated using the Account object, the new
location won't be correct in the e-mail because the save for that
field's model hasn't been called yet (if I understand things
correctly).

The current patch adds a new signal m2m_changed. An "action" argument
is sent to the handler telling it if it is an "add," "remove,"
"clear." This is obviously an addition to the other signals but it
seems rather clunky although Malcolm requested that [2]. and that it
would be better to allow the current signals to be called in a truly
"post_save" context, where the ManyToManyField models have been saved
(where appropriate: this probably only applies to the signals
post_save and maybe post_delete).

I see it has one transaction since there is more than one update going
on the use case, so a signal might be named "post_transaction" and you
receive the saved model that has its ManyToManyField's updated. Now, I
think it's more elegant and natural to just have the post_save (and
maybe post_delete) signal send the model instance with the
ManyToManyFields also saved.

I ran into this problem trying to cache data from the DB after a model
was saved that had ManyToMany fields and it never worked. I don't
think many people run into this so perhaps maybe it isn't pressing, or
maybe it's not possible to do, but the ticket said it was trying to
make it into 1.2. I don't think it addresses the problem with signals
not being able to be fully used and seems like a bandage. Then I think
about GenericForeignKeys where is wouldn't work either. I hope this
clears up the confusion (or reveals my own misunderstanding!).

Cheers,
Ryan

[1] http://groups.google.com/group/django-users/browse_thread/thread/82d5fb7694dc9c5e/350c941c5f81a22a
[2] http://groups.google.com/group/django-developers/browse_thread/thread/afe6ad7994d868ba

In Ticket 5390 there are a few patches attached to it

[Russell Keith-Magee]

On Fri, Dec 4, 2009 at 3:44 PM, Ryan K <mob...@ryankaskel.com> wrote:
> Sorry for the confusion but I sent the original link from my mobile
> handset which isn't great for copying/pasting/etc.

In future, could I please ask you to consider that when you send a
message to django-developers, it arrives in over 5000 mailboxes. If a
mobile device won't allow you to express yourself clearly, perhaps you
should wait until you have access to a suitable device.

> In another post, we were discussing the issue of ManyToMany updates
> [1] and how the signal handler for a model X that has a
> ManyToManyField to Y will _not_ have an updated Y. It's not
> necessarily a "bug" but its feature that's lacking and it's been in
> discussion for many years. Here is the use case from the ticket:
>
> For example, I want to send en e-mail when an Account is created to
> Account.email. An account has a ManyToManyField to a Location model
> and I want to for whatever reason include the location(s) in the e-
> mail. If the location was updated using the Account object, the new
> location won't be correct in the e-mail because the save for that
> field's model hasn't been called yet (if I understand things
> correctly).
>
> The current patch adds a new signal m2m_changed. An "action" argument
> is sent to the handler telling it if it is an "add," "remove,"
> "clear." This is obviously an addition to the other signals but it
> seems rather clunky although Malcolm requested that [2].

Malcolm requested this because it's just about the only way m2m
signals *can* work.

Saving a model and saving an m2m relation are two completely separate
operations. In fact, as of a couple of weeks ago, they're completely
different *models* - the m2m relation is managed using an
autogenerated intermediate model.

You can save a model without modifying the m2m, and you can save m2m
relations without touching the base model. The two operations are
completely separate. Outside of forms (which are only one use case of
models), there simply isn't any concept of a 'transaction' or a
'complete post_save'.

I appreciate that this makes certain types of assertions regarding
data integrity difficult, but this is entirely function of the way
Django's models are structured.

If you want to convince us that we should be taking a different
approach with m2m signals, you're going to need to back up your
suggestions with code that proves that your approach is actually
possible.

Yours,
Russ Magee %-)

[Ryan K]

Okay, thank you for clarifying the problem. I admit that I am least
familiar with the database operations as they are implemented in
Django so I will read the relevant code and see if indeed something
_can_ be done. This is not a bug at all but I think it raises a valid
question. Signals are really useful when used properly (I speculate
that some people would abuse the facility) and when you want to have
the ability to cache certain data (among other uses) it would be nice
to have an atomic operation (also what about a model with more than
one ManyToManyField -- is there a defined order in which they are
committed to the DB?).

I hope I'm not missing something glaringly obvious but the idea would
be that an option in the Django instance's settings would enable this
feature (or maybe as a decorator for each model or explicitly giving
the model a special manager). A manager could keep a reference of how
many ManyToMany relations it has. A post_save signal is then called
after the final save is called on the ManyToManyField reference
object. This would be a SerialCommitManager or something.

What if create_many_related_manager(superclass, rel=False) in [1]
could use one boolean setting's value so a temporary special manager
is created for the ManyToMany relation. One or more of these special
manager objects talk to a the parent Manager who's model is that which
has a post_save signal handler registered. Surely this would be much
slower but for situations where write's are very frequent it's okay to
pay. Again, I'm a history major and I'm trying to learn computer
science through application so this functionality is just an exercise
in seeing whether or not I can do this since it drove me crazy for a
while. I am finishing up school in the next three weeks so I have a
lot to do before graduation but I will definitely see if there is an
elegant way to do this. Thanks much for all your help and do tell me
if my thinking is way off!!

Cheers,
Ryan Kaskel

[1] http://code.djangoproject.com/browser/django/trunk/django/db/models/fields/related.py

[Ryan K]

Correction: "Surely this would be much slower but for situations where
write's are very [IN]frequent it's okay to pay."

[Russell Keith-Magee]

On Sat, Dec 5, 2009 at 5:35 AM, Ryan K <mob...@ryankaskel.com> wrote:
>
> I hope I'm not missing something glaringly obvious but the idea would
> be that an option in the Django instance's settings would enable this
> feature (or maybe as a decorator for each model or explicitly giving
> the model a special manager).

Stepping aside from the fact that class decorators are only available
in Django 3.0...

> A manager could keep a reference of how
> many ManyToMany relations it has. A post_save signal is then called
> after the final save is called on the ManyToManyField reference
> object. This would be a SerialCommitManager or something.

Consider a model A with 2 m2m fields, b and c, plus an m2 field d that
is defined on a different model. Now, consider the following sequence
of operations.

instance = A()
instance.attr = 1
instance.save()
instance.b = [...]
instance.attr = 2
instance.save()
instance.c = [...]
instance.attr = 3
instance.d = [...]
instance.save()

At which point in this sequence is your "complete" signal sent?

Yours
Russ Magee %-)

[Russell Keith-Magee]

On Sat, Dec 5, 2009 at 10:56 AM, Russell Keith-Magee
<freakb...@gmail.com> wrote:
> On Sat, Dec 5, 2009 at 5:35 AM, Ryan K <mob...@ryankaskel.com> wrote:
>>
>> I hope I'm not missing something glaringly obvious but the idea would
>> be that an option in the Django instance's settings would enable this
>> feature (or maybe as a decorator for each model or explicitly giving
>> the model a special manager).
>
> Stepping aside from the fact that class decorators are only available
> in Django 3.0...

Of course, that should read _Python_ 3.0.

Yours,
Russ Magee %-)