Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add note to documentation of how to relate to ProcrastinateJob model from another Django model #1138

Closed
medihack opened this issue Jul 31, 2024 · 3 comments · Fixed by #1191
Labels
Issue appropriate for: newcomers 🤩 This issue can be safely tackled by people who never worked in this project before Issue contains: Some documentation 📚 This issues involves writing some documentation PR type: documentation 📚 Contains documentation updates

Comments

@medihack
Copy link
Member

When creating a relation from another Django model to the ProcrastinateJob model like this

queued_job = models.OneToOneField(
  ProcrastinateJob, null=True, on_delete=models.SET_NULL, related_name="+"
)

I get an error when jobs are deleted from the database (e.g., by app.run_worker(delete_jobs="always")). This is because Django sets the on_delete trigger only at the application level but not at the database level.

I solved it by altering my table itself with some custom migration. I even created a function for that

from string import Template

def procrastinate_on_delete_sql(app_name: str, model_name: str, reverse=False):
    template = """
        ALTER TABLE ${app_name}_${model_name}
        DROP CONSTRAINT ${app_name}_${model_name}_queued_job_id_key,
        ADD CONSTRAINT ${app_name}_${model_name}_queued_job_id_key
        FOREIGN KEY (queued_job_id) 
        REFERENCES procrastinate_jobs(id) 
        """

    if not reverse:
        template += "\nON DELETE SET NULL;"

    return Template(template).substitute(app_name=app_name, model_name=model_name)

I think we should add a note to the documentation (for example, in the FAQ).

@medihack medihack added Issue appropriate for: newcomers 🤩 This issue can be safely tackled by people who never worked in this project before Issue contains: Some documentation 📚 This issues involves writing some documentation PR type: documentation 📚 Contains documentation updates labels Jul 31, 2024
@medihack medihack changed the title Add note to FAQ of how to relate to ProcrastinateJob model from another Django model Add note to documentation of how to relate to ProcrastinateJob model from another Django model Jul 31, 2024
@ewjoachim
Copy link
Member

If we document it, I think it makes more sense to document it as a migration and let people factor it differently if relevant for them. For the vast majority of people, the relevant thing to do will be to create a migration.
Also, if you create the FK (or o2o) with db_constraint=False, you don't even need to remove the constraint.

@ewjoachim
Copy link
Member

BTW, here's how Django generates the constraint name

@medihack
Copy link
Member Author

medihack commented Aug 1, 2024

If we document it, I think it makes more sense to document it as a migration and let people factor it differently if relevant for them. For the vast majority of people, the relevant thing to do will be to create a migration.

Sure, it was only so that I don't have to look up the SQL later 😉.

Also, if you create the FK (or o2o) with db_constraint=False, you don't even need to remove the constraint.

Thanks, good to know!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Issue appropriate for: newcomers 🤩 This issue can be safely tackled by people who never worked in this project before Issue contains: Some documentation 📚 This issues involves writing some documentation PR type: documentation 📚 Contains documentation updates
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants