43

There are many Stack Overflow posts about recursion using the post_save signal, to which the comments and answers are overwhelmingly: "why not override save()" or a save that is only fired upon created == True.

Well I believe there's a good case for not using save() - for example, I am adding a temporary application that handles order fulfillment data completely separate from our Order model.

The rest of the framework is blissfully unaware of the fulfillment application and using post_save hooks isolates all fulfillment related code from our Order model.

If we drop the fulfillment service, nothing about our core code has to change. We delete the fulfillment app, and that's it.

So, are there any decent methods to ensure the post_save signal doesn't fire the same handler twice?

Yuji 'Tomita' Tomita
  • 115,817
  • 29
  • 282
  • 245
  • Is this just a rumination, or do you have a specific situation where you've observed multiple firings of the same handler for a single event? E.g. `save()` fires `post_save`, which causes some *other* handler to `save()` the same object again and thus cause another firing of `post_save`? – Peter Rowell May 31 '12 at 19:44
  • 1
    @PeterRowell, yes, if your post_save handler needs to save the sender instance, it will trigger post_save again. – Yuji 'Tomita' Tomita May 31 '12 at 20:18
  • read here with bulk_create you can save without postsave and presave call [django docs](https://docs.djangoproject.com/en/dev/ref/models/querysets/#django.db.models.query.QuerySet.bulk_create) – mujad Jun 22 '19 at 11:53

11 Answers11

94

you can use update instead of save in the signal handler

queryset.filter(pk=instance.pk).update(....)
Nico Griffioen
  • 5,143
  • 2
  • 27
  • 36
mossplix
  • 3,783
  • 2
  • 26
  • 31
43

What you think about this solution?

@receiver(post_save, sender=Article)
def generate_thumbnails(sender, instance=None, created=False, **kwargs):

    if not instance:
        return

    if hasattr(instance, '_dirty'):
        return

    do_something()

    try:
        instance._dirty = True
        instance.save()
    finally:
        del instance._dirty

You can also create decorator

def prevent_recursion(func):

    @wraps(func)
    def no_recursion(sender, instance=None, **kwargs):

        if not instance:
            return

        if hasattr(instance, '_dirty'):
            return

        func(sender, instance=instance, **kwargs)

        try:
            instance._dirty = True
            instance.save()
        finally:
            del instance._dirty

    return no_recursion


@receiver(post_save, sender=Article)
@prevent_recursion
def generate_thumbnails(sender, instance=None, created=False, **kwargs):

    do_something()
yspreen
  • 1,759
  • 2
  • 20
  • 44
xakdog
  • 655
  • 6
  • 12
  • Wow, pretty simple and yet smart solution, I think this is the best answer so far. I wish there were more comments on this. – eneepo Jun 27 '15 at 14:15
  • Can somebody explain what's happening in this solution? I'm struggling to find any documentation for the _dirty flag – Max Mumford Jan 08 '16 at 15:15
  • 2
    @Max Mumford It is not an flag. To prevent recursion we can set custom attribute before `instance.save()` with any unused name like `_dirty`. Then check if attribute exists and break the recursion.As you can see we set `_dirty` attribute before we save the model and remove it when saving is done. – xakdog Jan 10 '16 at 20:20
  • Oops, of course, my bad; I should have looked more carefully at the rest of the function instead of being in a hurry. Thanks 4ell – Max Mumford Jan 12 '16 at 10:42
  • 5
    It should be the chosen answer. – khamaileon May 01 '16 at 09:35
  • 3
    Would be even better with a `try...finally` around the call to `instance.save()` to ensure that `del instance._dirty` is always run, even in case of e.g. `DatabaseError`. – Day Aug 18 '16 at 15:16
  • Does this cause a race condition in the case of multiple calls to save with a single instance? – Marviel Dec 10 '18 at 14:56
  • 1
    It might help to show the import `from functools import wraps` – shanemgrey May 03 '19 at 17:32
38

Don't disconnect signals. If any new model of the same type is generated while the signal is disconnected the handler function won't be fired. Signals are global across Django and several requests can be running concurrently, making some fail while others run their post_save handler.

punkgode
  • 493
  • 4
  • 6
  • That's the point. We don't want the signals to run while we are inside the signals. Besides, signals are not asynchronous. Check out https://stackoverflow.com/questions/11899088/is-django-post-save-signal-asynchronous – Bobort Sep 18 '18 at 22:23
  • 1
    Several requests are not handled by the same thread. Signals handlers block only in the thread they are running, that means that if you disconnect a signal handler while OTHER request gets processed, it's signal handlers won't fire. I can't think of a single scenario while you'd want that. P.S. 2018 wow – punkgode Mar 25 '20 at 02:19
  • Mmm, if we use transaction atomic we don't lose any signal, right? https://docs.djangoproject.com/en/3.0/topics/db/transactions/#controlling-transactions-explicitly – mrroot5 Jul 22 '20 at 09:11
  • 1
    @mrroot5 AFAIK no, that only assures atomic DB operations, not signal handling. If you disconnect a signal you will loose any signal that is be fired while its disconnected. – punkgode Jul 23 '20 at 12:49
28

I think creating a save_without_signals() method on the model is more explicit:

class MyModel()
    def __init__():
        # Call super here.
        self._disable_signals = False

    def save_without_signals(self):
        """
        This allows for updating the model from code running inside post_save()
        signals without going into an infinite loop:
        """
        self._disable_signals = True
        self.save()
        self._disable_signals = False

def my_model_post_save(sender, instance, *args, **kwargs):
    if not instance._disable_signals:
        # Execute the code here.
Rune Kaagaard
  • 6,643
  • 2
  • 38
  • 29
  • 4
    I like it. One suggestion though: instead of overriding `__init__` you could just do `if not getattr(instance, '_disable_signals', False)` – dgel Oct 22 '14 at 20:39
  • 3
    Is this thread safe? What happens if a regular save is triggered at the same time as a save_without_signals is happening? – Max Mumford Jan 08 '16 at 15:14
  • 1
    Yes it's thread safe. The ``_disable_signals`` property is local to each model instance. – Rune Kaagaard Jan 10 '16 at 19:39
  • 3
    Like it. Since the second calling of post save signal does nothing, I hope django will add a parameter in save function, so we can use `instance.save(send_signal=False)`. – ramwin Aug 03 '17 at 03:54
  • 1
    @RuneKaagaard Do you have a reference for `_disable_signals` being local to each model instance? This is fantastic. But I'd love to read/learn more about it. – culix Oct 02 '19 at 18:49
  • @culix It's just how Python works. See: https://stackoverflow.com/a/5690920/164449 – Rune Kaagaard Nov 19 '19 at 17:12
19

How about disconnecting then reconnecting the signal within your post_save function:

def my_post_save_handler(sender, instance, **kwargs):
    post_save.disconnect(my_post_save_handler, sender=sender)
    instance.do_stuff()
    instance.save()
    post_save.connect(my_post_save_handler, sender=sender)
post_save.connect(my_post_save_handler, sender=Order)
dgel
  • 16,352
  • 8
  • 58
  • 75
  • I had read about this method, and wondered if there was a cleaner solution... I tried leaving attributes on the object being saved to detect if the handler had modified the object once, but it doesn't seem to persist. Any other options? – Yuji 'Tomita' Tomita May 31 '12 at 20:20
  • I don't believe there is any clean solution. – dgel May 31 '12 at 20:58
  • 6
    hah - I googled my own question and just used this again :) Thanks! – Yuji 'Tomita' Tomita Aug 08 '12 at 21:50
  • so when you reconnect, if you have code below the "post_save.connect" in the my_post_save_save_handler, it'll run the rest of the code? – dtc Feb 12 '13 at 23:52
  • 32
    this is dangerous, while your signal is disconnected instances could be saved and now your handler will be missed. – Daniel Jun 18 '14 at 09:17
  • 1
    This is infact dangerous, so I am going with the update solution as I have used before without issue. – radtek Oct 22 '14 at 18:54
  • This helped me again and I cannot +1 it one more time. The update solution doesn't work when you want to update a ManyToMany field. – nima Jan 18 '15 at 20:01
  • 2
    This solution is not thread-safe, so concurrent requests can potentially either falsely re-invoke the signal for the save call inside "my_post_save_handler" or falsely not invoke the "my_post_save_handler" signal in the first place when called outside of ""my_post_save_handler". Would not recommend. – Rune Kaagaard Jun 03 '15 at 11:59
  • 3
    Not only is this not thread-safe, it is also missing a `try...finally`. Must do the (re)`connect` inside `finally` clause. Without that, if `do_stuff` or `save` raise an exception then your handler will stay disconnected forever... – Day Aug 18 '16 at 15:10
4

You should use queryset.update() instead of Model.save() but you need to take care of something else:

It's important to note that when you use it, if you want to use the new object you should get his object again, because it will not change the self object, for example:

>>> MyModel.objects.create(pk=1, text='')
>>> el = MyModel.objects.get(pk=1)
>>> queryset.filter(pk=1).update(text='Updated')
>>> print el.text
>>> ''

So, if you want to use the new object you should do again:

>>> MyModel.objects.create(pk=1, text='')
>>> el = MyModel.objects.get(pk=1)
>>> queryset.filter(pk=1).update(text='Updated')
>>> el = MyModel.objects.get(pk=1) # Do it again
>>> print el.text
>>> 'Updated'
ruhanbidart
  • 4,564
  • 1
  • 26
  • 13
4

You could also check the raw argument in post_save and then call save_baseinstead of save.

dragoon
  • 5,601
  • 5
  • 37
  • 55
1

the Model's .objects.update() method bypasses the post_save signal

Try this something like this:

from django.db import models
from django.db.models.signals import post_save


class MyModel(models.Model):

    name = models.CharField(max_length=200)
    num_saves = models.PositiveSmallIntegerField(default=0)

    @classmethod
    def post_save(cls, sender, instance, created, *args, **kwargs):
        MyModel.objects.filter(id=instance.id).update(save_counter=instance.save_counter + 1)

post_save.connect(MyModel.post_save, sender=MyModel)

In this example, an object has a name and each time .save() is called, the .num_saves property is incremented, but without recursion.

Adonis Gaitatzis
  • 3,189
  • 26
  • 24
0

Check this out...

Each signal has it's own benefits as you can read about in the docs here but I wanted to share a couple things to keep in mind with the pre_save and post_save signals.

  • Both are called every time .save() on a model is called. In other words, if you save the model instance, the signals are sent.

  • running save() on the instance within a post_save can often create a never ending loop and therefore cause a max recursion depth exceeded error --- only if you don't use .save() correctly.

  • pre_save is great for changing just instance data because you do not have to call save() ever which eliminates the possibility for above. The reason you don't have to call save() is because a pre_save signal literally means right before being saved.

  • Signals can call other signals and or run delayed tasks (for Celery) which can be huge for usability.

Source: https://www.codingforentrepreneurs.com/blog/post-save-vs-pre-save-vs-override-save-method/

Regards!!

Jesús Díaz
  • 362
  • 4
  • 5
0

In post_save singal in django for avoiding recursion 'if created' check is required

from django.dispatch import receiver
from django.db.models.signals import post_save

@receiver(post_save, sender=DemoModel)
def _post_save_receiver(sender,instance,created, **kwargs):
    if created:            
       print('hi..')
       instance.save()
Tejsingh
  • 277
  • 2
  • 4
0

I was using the save_without_signals() method by @Rune Kaagaard until i updated my Django to 4.1. On Django 4.1 this method started raising an Integrity error on the database that gave me 4 days of headaches and i couldn't fix it.

So i started to use the queryset.update() method and it worked like a charm. It doesn't trigger the pre_save() neither post_save() and you don't need to override the save() method of your model. 1 line of code.

@receiver(pre_save, sender=Your_model)
def any_name(sender, instance, **kwargs):
    Your_model.objects.filter(pk=instance.pk).update(model_attribute=any_value)
Koedlt
  • 4,286
  • 8
  • 15
  • 33
José N.
  • 11
  • 1