Opened 16 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@… 16 years ago.
Patch against trunk using the send_signals=False method.
send_signals_false_patch_correct.diff (3.3 KB ) - added by brooks.travis@… 16 years ago.
Patch against trunk using the send_signals=False method.
signals_patch3.diff (2.7 KB ) - added by brooks.travis@… 16 years ago.
A new patch against trunk (8964) using the send_signals=False method.

Download all attachments as: .zip

Change History (17)

comment:1 by Michael Radziej, 16 years ago

milestone: post-1.0

comment:2 by Michael Radziej, 16 years ago

Triage Stage: UnreviewedDesign decision needed

comment:3 by anonymous, 16 years ago

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.

by brooks.travis@…, 16 years ago

Patch against trunk using the send_signals=False method.

by brooks.travis@…, 16 years ago

Patch against trunk using the send_signals=False method.

comment:4 by brooks.travis@…, 16 years ago

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 by anonymous, 16 years ago

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)

by brooks.travis@…, 16 years ago

Attachment: signals_patch3.diff added

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

comment:6 by (none), 15 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:7 by Malcolm Tredinnick, 15 years ago

Component: Database layer (models, ORM)Core framework

comment:8 by Michel Sabchuk, 15 years ago

Cc: michel@… added

comment:9 by oliland, 14 years ago

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 by Luke Plant, 14 years ago

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 by davedash, 13 years ago

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 by EmilStenstrom, 13 years ago

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 by EmilStenstrom, 13 years ago

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 by Luke Plant, 13 years ago

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