Disable recalculation on delete

[striveforbest]

Is there a way to disconnect signal or disable recalculation on .delete()?

I have a Work model with a reverse fk to Price

class Work(TimeStampedModel, ComputedFieldsModel):
    @computed(
        models.ForeignKey(to=Price, null=True, default=None, on_delete=models.SET_NULL, related_name='+', verbose_name='Computed Current Price'),
        depends=[
            ['_prices', ['price']],
        ],
    )
    def computed_current_price(self):
        """
        Computed field that stores most recent price object's id used for search.
        """
        return self.prices.first()

When I try to delete the work, it fails, see full traceback:

In [1]: Work.objects.get(id=163860).delete()
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/db/models/fields/related_descriptors.py in __get__(self, instance, cls)
    172         try:
--> 173             rel_obj = self.field.get_cached_value(instance)
    174         except KeyError:

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/db/models/fields/mixins.py in get_cached_value(self, instance, default)
     14         try:
---> 15             return instance._state.fields_cache[cache_name]
     16         except KeyError:

KeyError: 'computed_current_price'

During handling of the above exception, another exception occurred:

DoesNotExist                              Traceback (most recent call last)
<ipython-input-1-464fd4e787fa> in <module>
----> 1 Work.objects.get(id=163860).delete()

~/projects/gestalt/noya/gagosian/works/models.py in delete(self, **kwargs)
   1474         # PHP also removes this work from scratchpads, which we do not use.
   1475
-> 1476         super().delete(**kwargs)
   1477
   1478     # Utility

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/db/models/base.py in delete(self, using, keep_parents)
    945         collector = Collector(using=using)
    946         collector.collect([self], keep_parents=keep_parents)
--> 947         return collector.delete()
    948
    949     delete.alters_data = True

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/db/models/deletion.py in delete(self)
    434                     for obj in instances:
    435                         signals.post_delete.send(
--> 436                             sender=model, instance=obj, using=self.using
    437                         )
    438

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/dispatch/dispatcher.py in send(self, sender, **named)
    177         return [
    178             (receiver, receiver(signal=self, sender=sender, **named))
--> 179             for receiver in self._live_receivers(sender)
    180         ]
    181

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/dispatch/dispatcher.py in <listcomp>(.0)
    177         return [
    178             (receiver, receiver(signal=self, sender=sender, **named))
--> 179             for receiver in self._live_receivers(sender)
    180         ]
    181

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/computedfields/handlers.py in postdelete_handler(sender, instance, **kwargs)
    106         with transaction.atomic():
    107             for model, [pks, fields] in updates.items():
--> 108                 active_resolver.bulk_updater(model.objects.filter(pk__in=pks), fields)
    109
    110

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/computedfields/resolver.py in bulk_updater(self, queryset, update_fields, return_pks, local_only)
    611                 for comp_field in mro:
    612                     new_value = self._compute(elem, model, comp_field)
--> 613                     if new_value != getattr(elem, comp_field):
    614                         has_changed = True
    615                         setattr(elem, comp_field, new_value)

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/db/models/fields/related_descriptors.py in __get__(self, instance, cls)
    185                 rel_obj = None
    186             if rel_obj is None and has_value:
--> 187                 rel_obj = self.get_object(instance)
    188                 remote_field = self.field.remote_field
    189                 # If this is a one-to-one relation, set the reverse accessor

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/db/models/fields/related_descriptors.py in get_object(self, instance)
    152         qs = self.get_queryset(instance=instance)
    153         # Assuming the database enforces foreign keys, this won't fail.
--> 154         return qs.get(self.field.get_reverse_related_filter(instance))
    155
    156     def __get__(self, instance, cls=None):

~/venv/noya-4UEHQ2KQ/lib/python3.7/site-packages/django/db/models/query.py in get(self, *args, **kwargs)
    429             raise self.model.DoesNotExist(
    430                 "%s matching query does not exist." %
--> 431                 self.model._meta.object_name
    432             )
    433         raise self.model.MultipleObjectsReturned(

DoesNotExist: Price matching query does not exist.

[jerch]

What type is self.prices? Is this a relation manager? If so, then the related objects in prizes prolly already got de-associated/deleted and .first() will fail. You can try with an exception handler around:

try:
    return self.prices.first()
except Price.DoesNotExist:
    return None  # field must allow NULL values

ForeignKey fields as computed fields are somewhat tricky to handle, as they might expose partially deleted intermediate states of the ORM in the computed method.

Note that simply skipping CF updates on delete is not an option, because CFs can depend on each other. By deliberately skipping updates follow-up CFs would not get notified/updated anymore. (Example - in your case another CF might want to sum over all Work.computed_current_price, thus has to be informed, that an item got removed to correctly remove that value.)

[striveforbest]

What type is self.prices? Is this a relation manager? If so, then the related objects in prizes prolly already got de-associated/deleted and .first() will fail. You can try with an exception handler around:

Price is a reverse fk to Work, so m2m from Work:

class Price(UpdateRelatedWorkModifiedTimeMixin, TimeStampedModel, models.Model):
    work = models.ForeignKey('Work', models.CASCADE, verbose_name='Work', related_name='_prices')

Try/except doesn't work. The issue is that Work.delete triggers CASCADE delete of its related Sale objects:

class Sale(UpdateRelatedWorkModifiedTimeMixin, TimeStampedModel, models.Model):
    work = models.ForeignKey(Work, models.CASCADE, related_name='sales')
    _price = models.OneToOneField(Price, models.RESTRICT)

Which then triggers it's post_delete signal handler to clean up restricted _price:

@receiver(post_delete, sender=Sale, dispatch_uid='delete_sales_price')
def delete_sale_price(sender, instance, using, **kwargs):
    # Delete the underlying, protected, price after the sale has been deleted
    instance._price.delete()

And the above triggers recalculation of Work.computed_current_price.

Note that simply skipping CF updates on delete is not an option, because CFs can depend on each other. By deliberately skipping updates follow-up CFs would not get notified/updated anymore. (Example - in your case another CF might want to sum over all Work.computed_current_price, thus has to be informed, that an item got removed to correctly remove that value.)

In my case, I am deleting the whole work and CASCADE deleting its relations. so shouldn't matter?

Disconnecting the computedfields.handlers.predelete_handler in Work.delete() worked but feels wrong and dirty.

[jerch]

Sorry, cannot follow the issue with that scattered description. Can you post a downstripped model declaration, that repros the issue?

In my case, I am deleting the whole work and CASCADE deleting its relations. so shouldn't matter?

Thats true, but the CF resolver does not know, if there are follow-up deps to be updated at a particular tree node during runtime. There is no look-ahead (thats not even doable due to possible model-field recursions).

Disconnecting the computedfields.handlers.predelete_handler in Work.delete() worked but feels wrong and dirty.

Yes thats not a good idea, as it cuts whole update subtrees with possible worse side effects.

[jerch]

@StriveForBest Any updates on this? Plz write a short repro that I can use to test against.

Again I am not sure if we can get ForeignKey working to full extend in CFs, as they add another level of indirection and partially degraded interim states. Will see.

[striveforbest]

I ended up refactoring the code to avoid our custom signals. But it does seem like an issue. We can close for now.