Sending create_historical_record() to celery tasks

[cromig01]

Problem Statement To minimize performance concerns, I want to send create_historical_record() as a celery task. This way we can easily handle one-off scripts that update a bunch of records, as well as avoiding database overloading if there's a bunch of historical records trying to be created and the db is locked.

I was reading up on this thread, which suggests that we can override the create_historical_record and create a custom method that is sent to celery. How would we go about overriding this method?

Describe the solution you'd like A relatively straightforward solution to overriding the create_historical_record so we can send it as a job, instead of writing to the db at runtime

Describe alternatives you've considered I've read about monkey patching, which would be instantiating the customized create_historical_record() at runtime and overriding the original method.

[ddabble]

I have no experience with Celery, but simply overriding HistoricalRecords.create_historical_record() can be done using standard class extension + method overriding:

from typing import override
from django.db import models
from simple_history.models import HistoricalRecords

class HistoricalRecordsWithCelery(HistoricalRecords):
    @override  # If using Python >= 3.12
    def create_historical_record(self, instance, history_type, using=None):
        # custom code (invoking Celery?)

class YourHistoryTrackedModel(models.Model):
    ...

    history = HistoricalRecordsWithCelery()

Does that work for you? 🙂

[cromig01]

@ddabble thank you for the speedy response! My project is currently on Python 3.10, so I'm not sure if @override will work for me. Is that just an annotation? I also tested out the class extension suggestion, but I'm noticing two issues:

1. I don't want to make HistoricalRecordsWithCelery a new model, which would happen with this approach
2. I have a few attributes I'm passing into the HistoricalRecords class (ex: use_base_model_db=False), which doesn't seem to work if I'm using the class as a parent class. I'm getting a TypeError: 'HistoricalRecords' object is not callable err

[cromig01]

I tested out something similar to this solution in my apps.py, which worked for me when testing:

HistoricalRecords.create_historical_record = MethodType(self.create_historical_record, HistoricalRecords)

This overrides the create_historical_record() method at runtime

[ddabble]

Yeah, @override is just an annotation that simply helps your IDE/editor analyze your code and give feedback on it; it has no effect when actually running the code 🙂

1. I don't want to make HistoricalRecordsWithCelery a new model, which would happen with this approach

Could you help me understand how that would happen? 🤔 Because I would instead describe HistoricalRecords (and, by extension, HistoricalRecordsWithCelery) as a manager, since you can use it pretty much the same way as you would use the default manager objects. E.g. both YourModel.objects.filter(...) and YourModel.history.filter(...) (where history is an instance of HistoricalRecords) work in the way you would expect 🙂

2. I have a few attributes I'm passing into the HistoricalRecords class (ex: use_base_model_db=False), which doesn't seem to work if I'm using the class as a parent class. I'm getting a TypeError: 'HistoricalRecords' object is not callable err

I'm not sure I understand how you're getting that error; could you provide a minimally reproducible example?

---

HistoricalRecords.create_historical_record = MethodType(self.create_historical_record, HistoricalRecords)

This overrides the create_historical_record() method at runtime

Sure, that could also work :) Though there are reasons monkey-patching is generally regarded as a last resort, some of which include that it might produce unexpected side effects (to developers who are not aware that it was monkey-patched) and hard-to-trace errors, and that it's often much less flexible compared to "proper" solutions - which I think would be class inheritance, in this case.

[cromig01]

yes for sure! Here's what I tried doing, which resulted in TypeError: 'HistoricalRecords' object is not callable when running ./manage.py makemigrations:

class ExtraHistoricalClaimFieldsModel(models.Model):
    backend_path = models.CharField(
        max_length=200, blank=True, null=True, default=None
    )

    class Meta:
        abstract = True

class HistoricalBaseModel(HistoricalRecords(use_base_model_db=False, history_user_id_field=models.IntegerField(null=True), bases=[ExtraHistoricalClaimFieldsModel, ])):
    def create_historical_record(self, instance, history_type, using=None):
        # call create_historical_record.delay()
        return "saved"

class YourHistoryTrackedModel(models.Model):
    ...

    history = HistoricalBaseModel()

But I think your right, monkey patching should be used as a last resort since it would definitely be harder to debug

[ddabble]

Ah, I think I see what you're trying to do 😅 The class list inside the parentheses after a class name (i.e. class ClassName(class list)) should only contain references to other classes (e.g. HistoricalRecords or models.Model), not constructor calls (e.g. HistoricalRecords(args...)). And the arguments that you want to pass to HistoricalRecords should be passed in the same place you would normally pass them, but with HistoricalRecords replaced by the child class. So I think doing this should fix the error:

class ExtraHistoricalClaimFieldsModel(models.Model):
    backend_path = models.CharField(
        max_length=200, blank=True, null=True, default=None
    )

    class Meta:
        abstract = True

class CustomHistoricalRecords(HistoricalRecords):
    def create_historical_record(self, instance, history_type, using=None):
        # call create_historical_record.delay()
        return "saved"

class YourHistoryTrackedModel(models.Model):
    ...

    history = CustomHistoricalRecords(use_base_model_db=False, history_user_id_field=models.IntegerField(null=True), bases=[ExtraHistoricalClaimFieldsModel, ])

Does that make sense? 🙂

[cromig01]

ahhh yay that worked! 🥳 makes sense that I should be putting the constructors in the constructor method instead of where the class is being inherited. thanks for the help :)

[ddabble]

Alright nice, no problem :)

Closing, now that the issue seems resolved.