Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add ignore_saving_historical_record_on_delete flag #736

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pvanderw
Copy link

@pvanderw pvanderw commented Nov 6, 2020

Description

Add ignore_saving_historical_record_on_delete flag on HistoricalRecord model which ignores saving history when a model is deleted. Fixes performance issue when cascading deletions create a large number of history instances.

Related Issue

Closes #642 and closes #993.
This relates to #717, #858.

Motivation and Context

Deleting a model that causes a lot of cascading deletions is slow since it creates a new history instance for each delete, potentially causing thousands of database queries.

How Has This Been Tested?

Added new unit tests with the flag set to both True/False

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run the make format command to format my code
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have added my name and/or github handle to AUTHORS.rst
  • I have added my change to CHANGES.rst
  • All new and existing tests passed.

…d model which ignores saving history when a model is deleted. Fixes performance issue when cascading deletions create a large number of history instances.
@krid
Copy link

krid commented Sep 21, 2021

Any news on this? Looks like it needs just a bit of work to push it over the line.

I'd do it myself, but my company has insanely restrictive policies around open source contributions.

Copy link
Contributor

@jeking3 jeking3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a good idea to re-do this pull request in the same manner that skip_history_when_saving (and save_without_historical_record) was implemented. It would be better as a per-operation choice and not a table-wide choice. I do appreciate the original pull request came with tests though!

@jeking3
Copy link
Contributor

jeking3 commented Sep 22, 2021

@krid you do have a personal GitHub account too, right?

@krid
Copy link

krid commented Sep 22, 2021

@krid you do have a personal GitHub account too, right?

I do, but the name on the account doesn't change who I am. Furthermore, if I work on this in my free time I'm just donating time & money to my employer, and they have enough of my life as it is.

@jeking3
Copy link
Contributor

jeking3 commented Oct 6, 2021

Similar to #642

@ddabble ddabble mentioned this pull request Sep 10, 2024
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants