Opened 15 years ago

Closed 13 years ago

#8077 closed (wontfix)

Add ability to disable signals one a per-object-per-event basis.

Reported by: brooks.travis@… Owned by: nobody
Component: Core (Other) Version: dev
Severity: Keywords: signals
Cc: brooks.travis@…, michel@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Right now, built-in signals like pre_save and post_save are triggered for all models every time a new or existing instance is saved to the database. I would propose adding the capability to granularly disable any built-in signals on a per-event basis. Here's an example syntax:

>>>MyModel(name='John Doe', disabled_signals=['post_save'])

You should be able to pass this argument to a ModelForm instance, as well, and you could probably make it work for custom signals, but that's probably less necessary.

Adding the capability would make working with the build-in signals a lot less of a headache, particularly in my use-case, where a modification to one model with a signal listener attached, triggers a change to another model with a signal listener attached, but I don't want latter listener triggered every time the former is. Let me know if I'm not being clear.

Attachments (3)

send_signals_false_patch.diff (3.2 KB) - added by brooks.travis@… 15 years ago.
Patch against trunk using the send_signals=False method.
send_signals_false_patch_correct.diff (3.3 KB) - added by brooks.travis@… 15 years ago.
Patch against trunk using the send_signals=False method.
signals_patch3.diff (2.7 KB) - added by brooks.travis@… 15 years ago.
A new patch against trunk (8964) using the send_signals=False method.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 15 years ago by Michael Radziej

milestone: post-1.0

comment:2 Changed 15 years ago by Michael Radziej

Triage Stage: UnreviewedDesign decision needed

comment:3 Changed 15 years ago by anonymous

Alternative api implementation, since I would imagine the most common use-case would be to suppress all signals for a given object:

>>>MyModel(name='John Doe', send_signals=False)

Then, some conditionals in the base Model definitions could just test for the send_signals kwarg (True by default) before running dispatcher.send.

Changed 15 years ago by brooks.travis@…

Patch against trunk using the send_signals=False method.

Changed 15 years ago by brooks.travis@…

Patch against trunk using the send_signals=False method.

comment:4 Changed 15 years ago by brooks.travis@…

Has patch: set

The send_signals_false_patch_correct.diff is the one that is formatted properly. If an admin could delete the first patch that would be great! Thanks.

comment:5 Changed 15 years ago by anonymous

Correction to api for proposed patch:

Don't send pre/post_init signals:

>>>a = MyModel(name='John Doe', send_signals=False)

Don't send pre/post_save signals:

>>>a = MyModel(name='John Doe')
>>>a.save(send_signals=False)

Changed 15 years ago by brooks.travis@…

Attachment: signals_patch3.diff added

A new patch against trunk (8964) using the send_signals=False method.

comment:6 Changed 15 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:7 Changed 15 years ago by Malcolm Tredinnick

Component: Database layer (models, ORM)Core framework

comment:8 Changed 14 years ago by Michel Sabchuk

Cc: michel@… added

comment:9 Changed 13 years ago by oliland

Has there been any update on this? This gets a +1 from me, but I see it hasn't been updated in 11 months.

comment:10 Changed 13 years ago by Luke Plant

It's difficult to assess whether this is really needed without a concrete example. It could well be an inappropriate use of signals in the first place — if the signal should only sometimes be run, should it really ever be run via a signal?

comment:11 Changed 13 years ago by davedash

Can't your signal check for a flag that you set on your model instance and run accordingly?

def myhandler(sender, instance, kw):

if instance.skip_post_save:

return

comment:12 Changed 13 years ago by EmilStenstrom

lukeplant: Here's a realworld example:

I have a two models, dialog and comments, where the comment have a foreignkey to the dialog. To speed up the display of number of comments I have a denormalized field on dialog, called num_comments. Now, this needs to be updated whenever a comment is added or deleted, so I listen to post_save and post_delete:

# sender=Comment
def recalc_comments(sender, instance, created=False, **kwargs):
    dialog = instance.dialog
    dialog.num_comments = sender.public.filter(dialog=dialog).count()
    dialog.save()

But this means that save will be run twice, since save() inside the signal triggers the signal too. Adding disable_signals to the save inside the signal would solve the problem I think.

Please let me know if this is not the right way to do this kind of thing. Thanks.

comment:13 Changed 13 years ago by EmilStenstrom

No, sorry: Saving a dialog does OF COURSE not trigger the comment signal. It does only trigger the Dialog signal. Which still is something one might want to prevent, but which is not part of my "realworld example".

comment:14 Changed 13 years ago by Luke Plant

Resolution: wontfix
Status: newclosed

Given the kind of examples given so far, I think there is a misunderstanding about the purpose of signals.

As the documentation says, signals are to help decoupled applications know about what is going on elsewhere in the framework. As the person writing MyModel(params) or my_model.save(), you don't know what signals are attached to the model, so you don't know whether it is appropriate to turn them off. The proposal is therefore not granular enough - you really need to disable a single handler that is attached via signals, not all of them.

On the other hand, if you are creating instances of the model, you are more like to be the person in charge of the model class, and you can always override __init__() and save() to do some custom work, and you can even pass custom parameters that will control whether the work needs to be done or not.

In short, it sounds like, given the defined purpose of signals, it is the attached signal handler that needs to become more intelligent (like in davedash's suggestion), rather than the code that emits the signal. Disabling signals is just a quick fix that will work when you know exactly what handlers are attached to a signal, and it hides the underlying problem by putting the fix in the wrong place.

Note: See TracTickets for help on using tickets.
Back to Top