Added signals that runs when adding an object to a Many-To-One relation

[Oskar Persson]

Hi everyone.

I've created a pull request (8018) that adds the signals pre_add and post_add that are triggered when adding an object to a Many-To-One relation. The reason for this PR is to provide a simple (is there any other?) way to run custom code when ever adding an object to a Many-To-One relation. There already exists a signal for Many-To-Many fields so why not for Many-To-One?

What are your thoughts on this?

Thanks

Oskar Persson

[Paul Egges]

I thought a Many-To-One would not require an intermediate table, so it would be caught by pre-save and post-save on the object containing the Foreign Key. Is that not true?

Paul

--
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+unsubscribe@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/249d697b-f030-4112-a192-2c8510461f5b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Oskar Persson]

No, the save method isn't called unless you specify bulk=False

Den fredag 3 februari 2017 kl. 17:53:04 UTC+1 skrev mtnpaul:

I thought a Many-To-One would not require an intermediate table, so it would be caught by pre-save and post-save on the object containing the Foreign Key. Is that not true?

Paul

On Fri, Feb 3, 2017 at 8:28 AM, Oskar Persson <oskar....@polken.se> wrote:

Hi everyone.

I've created a pull request (8018) that adds the signals pre_add and post_add that are triggered when adding an object to a Many-To-One relation. The reason for this PR is to provide a simple (is there any other?) way to run custom code when ever adding an object to a Many-To-One relation. There already exists a signal for Many-To-Many fields so why not for Many-To-One?

What are your thoughts on this?

Thanks

Oskar Persson

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

[Paul Egges]

Good to know. 

Thanks,

Paul

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@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/e6e9a557-44f0-4489-bc11-baae40e35930%40googlegroups.com.

[Tim Graham]

#21461 proposes to add signals for QuerySet.update(). Would this solve your use case?

https://code.djangoproject.com/ticket/21461

[Oskar Persson]

It would. Though that seems like its trying to solve a bigger issue since then you probably need the updated objects, which I'm not including

[Tim Graham]

Since you created a ticket [0], I assume you feel there is value is adding this signal even if the QuerySet.update() signal is added? Could you explain the rationale?

What are the performance implications of additional (possibly redundant) signals? As Aymeric said on the pull request, "Performance should be a consideration here, pre_init and post_init is a very expensive historical mistake."

[0] https://code.djangoproject.com/ticket/27809

[Oskar Persson]

No, if the QuerySet.update() signals were to be implemented then these signals wouldn't be needed anymore. However, since there currently aren't any solutions for the QuerySet.update() signals that aren't a performance concern then the add signals could be a first step there. They don't solve all the problems that the update signals does but they solve some of them without having any real impact on performance. 

[Oskar Persson]

And we could also maybe add signals similar to pre_add and post_add that are sent when calling remove() and clear() on a relation set, getting a bit closer to QuerySet.update()

[Tim Graham]

Could you describe your use case in more detail? I feel like it would result in less code duplication to write signal handlers for Model.save() and QuerySet.update() rather than for remove(), clear(), add(), and set() (the more high level ways that an object could be updated). I'm not happy to add signals that will be redundant if QuerySet.update() signals are added. On the other hand, if you can argue that those can't be added, then maybe we could consider this.

[Oskar Persson]

My specific need for this at the moment is that i need to do some computation (including cache invalidation) whenever objects of model A are added to/deleted from model B. This could easily be done if I had signals for these events. And the only thing that I need to know is what instance of B that was updated (The query is only done on objects of A but they are all related to the same instance of B).

If the QuerySet.update were to send a signal with all updated objects it would only send the objects of A, not B. At least with the solutions that I've seen suggested. So my idea would be to use the add signals (and remove, clear, set) as I've suggested to at least send a signal with the instance of B. Then if we come up with a solution for QuerySet.update() we could also send a signal with all objects from A. So I'm not sure I think its redundant in that way.

[Tim Graham]

Tell me if I'm wrong because I haven't worked through things in detail but it seems to me that an "add" signal wouldn't catch the case where objects are added or delete using QuerySet.update(). For the update() signal to process objects of B, it looks like you would check the 'update_fields" kwarg for B's foreign key.

[Oskar Persson]

Yes, my PR does just that, it sends the signal when bulk=True i.e. when an QuerySet.update() will be used. And a more general update() signal I guess would have to look through the fields that it's updating and check if its a foreign key and then send the signals for those elements some how

[Tim Graham]

I was unclear. My point is that by listening to a Manager.add() signal, your cache invalidation handler won't catch cases where other code calls QuerySet.update(). If update signals were implemented and your code listened to that, there would be no need to add or listen to a Manager.add() signal because it uses QuerySet.update() or QuerySet.save().

[Oskar Persson]

That is correct, yes.