Summary

Add a management command to clean up orphaned data.

Review Request #13740 — Created April 17, 2024 and submitted April 23, 2024, 3:53 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

It will query each of the affected items to get a total count, and then
perform the actual deletion in batches. This allows us to show nice
progress bars, as well as make the command interruptible.

Testing Done

Populated my database with a bunch of data including screenshots,
file attachments, diffs, and change descriptions. Temporarily disabled
our new signal handler and deleted the review requests.

Ran with --show-counts-only and saw the expected counts for each type
of object.
Ran normally and watched it delete all the objects.
Ran again with --show-counts-only and saw that it no longer reported
any orphaned data.
Set the batch size low and verified that the progress bars worked
correctly.
Built and looked at the docs.

Commits

Summary	ID
Add a management command to clean up orphaned data. This change adds a new management command that will clean up various pieces of data in the database and filesystem that could have been left behind if administrators used the "Delete Permanently" action on a review request or deleted review requests via the admin UI. It will query each of the affected items to get a total count, and then perform the actual deletion in batches. This allows us to show nice progress bars, as well as make the command interruptible. Testing Done: - Populated my database with a bunch of data including screenshots, file attachments, diffs, and change descriptions. Temporarily disabled our new signal handler and deleted the review requests. Ran with --show-counts-only and saw the expected counts for each type of object. Ran normally and watched it delete all the objects. Ran again with --show-counts-only and saw that it no longer reported any orphaned data. - Set the batch size low and verified that the progress bars worked correctly. - Built and looked at the docs.	e45b6c7f2d5c4d59c0c67d381f2cbf62dffef994

Summary

Add a management command to clean up orphaned data.

This change adds a new management command that will clean up various pieces of data in the database and filesystem that could have been left behind if administrators used the "Delete Permanently" action on a review request or deleted review requests via the admin UI. It will query each of the affected items to get a total count, and then perform the actual deletion in batches. This allows us to show nice progress bars, as well as make the command interruptible. Testing Done: - Populated my database with a bunch of data including screenshots, file attachments, diffs, and change descriptions. Temporarily disabled our new signal handler and deleted the review requests. Ran with --show-counts-only and saw the expected counts for each type of object. Ran normally and watched it delete all the objects. Ran again with --show-counts-only and saw that it no longer reported any orphaned data. - Set the batch size low and verified that the progress bars worked correctly. - Built and looked at the docs.

e45b6c7f2d5c4d59c0c67d381f2cbf62dffef994

Issues

Description	From	Last Updated
continuation line over-indented for visual indent Column: 21 Error code: E127	reviewbot	April 17, 2024, 5:17 p.m.
We should go with "data" or "objects" here instead of "file attachments" to be more general. Same with the module …	maubin	April 18, 2024, 10:03 a.m.
This should be taken out.	maubin	April 18, 2024, 10:04 a.m.
This is missing a ::.	chipx86	April 23, 2024, 7:59 a.m.
I think this needs to be gettext_lazy.	chipx86	April 23, 2024, 8 a.m.
Small nit: One per line would help keep this readable/more maintainable if we need to expand upon this.	chipx86	April 23, 2024, 8 a.m.
Same here.	chipx86	April 23, 2024, 8 a.m.
Keyword-only arguments would help make this a bit more readable.	chipx86	April 23, 2024, 8:01 a.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/management/commands/clean-orphaned-data.py (Diff revision 1)
The issue has been dropped. Show all issues
```
continuation line over-indented for visual indent

Column: 21
Error code: E127
```

Commits:

	Summary	ID
	Add a management command to clean up orphaned data. This change adds a new management command that will clean up various pieces of data in the database and filesystem that could have been left behind if administrators used the "Delete Permanently" action on a review request or deleted review requests via the admin UI. It will query each of the affected items to get a total count, and then perform the actual deletion in batches. This allows us to show nice progress bars, as well as make the command interruptible. Testing Done: - Populated my database with a bunch of data including screenshots, file attachments, diffs, and change descriptions. Temporarily disabled our new signal handler and deleted the review requests. Ran with --show-counts-only and saw the expected counts for each type of object. Ran normally and watched it delete all the objects. Ran again with --show-counts-only and saw that it no longer reported any orphaned data. - Set the batch size low and verified that the progress bars worked correctly. - Built and looked at the docs.	4c068c4256f752f89fb54d60dec07f85cb52e0ac
	Add a management command to clean up orphaned data. This change adds a new management command that will clean up various pieces of data in the database and filesystem that could have been left behind if administrators used the "Delete Permanently" action on a review request or deleted review requests via the admin UI. It will query each of the affected items to get a total count, and then perform the actual deletion in batches. This allows us to show nice progress bars, as well as make the command interruptible. Testing Done: - Populated my database with a bunch of data including screenshots, file attachments, diffs, and change descriptions. Temporarily disabled our new signal handler and deleted the review requests. Ran with --show-counts-only and saw the expected counts for each type of object. Ran normally and watched it delete all the objects. Ran again with --show-counts-only and saw that it no longer reported any orphaned data. - Set the batch size low and verified that the progress bars worked correctly. - Built and looked at the docs.	f266db310c633d0835b5ef664459bc1ebc9df2a7

Diff:

Revision 2 (+678)

Show changes

	docs/manual/admin/sites/management-commands.rst
	reviewboard/dependencies.py
	reviewboard/reviews/management/commands/clean-orphaned-data.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/reviews/management/commands/clean-orphaned-data.py (Diff revision 2)

The issue has been resolved. Show all issues

We should go with "data" or "objects" here instead of "file attachments" to be more general. Same with the module docstring too.

david April 18, 2024, 10:04 a.m.

Leftovers from before I discovered that we had more than just file attachments.

reviewboard/reviews/management/commands/clean-orphaned-data.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This should be taken out.
```

Commits:

	Summary	ID
	Add a management command to clean up orphaned data. This change adds a new management command that will clean up various pieces of data in the database and filesystem that could have been left behind if administrators used the "Delete Permanently" action on a review request or deleted review requests via the admin UI. It will query each of the affected items to get a total count, and then perform the actual deletion in batches. This allows us to show nice progress bars, as well as make the command interruptible. Testing Done: - Populated my database with a bunch of data including screenshots, file attachments, diffs, and change descriptions. Temporarily disabled our new signal handler and deleted the review requests. Ran with --show-counts-only and saw the expected counts for each type of object. Ran normally and watched it delete all the objects. Ran again with --show-counts-only and saw that it no longer reported any orphaned data. - Set the batch size low and verified that the progress bars worked correctly. - Built and looked at the docs.	f266db310c633d0835b5ef664459bc1ebc9df2a7
	Add a management command to clean up orphaned data. This change adds a new management command that will clean up various pieces of data in the database and filesystem that could have been left behind if administrators used the "Delete Permanently" action on a review request or deleted review requests via the admin UI. It will query each of the affected items to get a total count, and then perform the actual deletion in batches. This allows us to show nice progress bars, as well as make the command interruptible. Testing Done: - Populated my database with a bunch of data including screenshots, file attachments, diffs, and change descriptions. Temporarily disabled our new signal handler and deleted the review requests. Ran with --show-counts-only and saw the expected counts for each type of object. Ran normally and watched it delete all the objects. Ran again with --show-counts-only and saw that it no longer reported any orphaned data. - Set the batch size low and verified that the progress bars worked correctly. - Built and looked at the docs.	c2b6a24dac4832e5b7a0f322948ce64ac2c83e8b

Diff:

Revision 3 (+672)

Show changes

	docs/manual/admin/sites/management-commands.rst
	reviewboard/dependencies.py
	reviewboard/reviews/management/commands/clean-orphaned-data.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

This looks good.

Some small suggestions to help with readability, but overall looks good.

As a suggestion for a possible feature, it might be nice to be able to get a list of the IDs for each type of object that's going to be deleted so it can be audited or manually dealt with prior to actually invoking a deletion. But not a blocker for this change by any means.

docs/manual/admin/sites/management-commands.rst (Diff revision 3)
The issue has been resolved. Show all issues
```
This is missing a ::.
```
reviewboard/reviews/management/commands/clean-orphaned-data.py (Diff revision 3)
The issue has been resolved. Show all issues
```
I think this needs to be gettext_lazy.
```

reviewboard/reviews/management/commands/clean-orphaned-data.py (Diff revision 3)

The issue has been resolved. Show all issues

Small nit: One per line would help keep this readable/more maintainable if we need to expand upon this.

reviewboard/reviews/management/commands/clean-orphaned-data.py (Diff revision 3)

The issue has been resolved. Show all issues

Same here.

reviewboard/reviews/management/commands/clean-orphaned-data.py (Diff revision 3)

The issue has been resolved. Show all issues

Keyword-only arguments would help make this a bit more readable.

Commits:

	Summary	ID
	Add a management command to clean up orphaned data. This change adds a new management command that will clean up various pieces of data in the database and filesystem that could have been left behind if administrators used the "Delete Permanently" action on a review request or deleted review requests via the admin UI. It will query each of the affected items to get a total count, and then perform the actual deletion in batches. This allows us to show nice progress bars, as well as make the command interruptible. Testing Done: - Populated my database with a bunch of data including screenshots, file attachments, diffs, and change descriptions. Temporarily disabled our new signal handler and deleted the review requests. Ran with --show-counts-only and saw the expected counts for each type of object. Ran normally and watched it delete all the objects. Ran again with --show-counts-only and saw that it no longer reported any orphaned data. - Set the batch size low and verified that the progress bars worked correctly. - Built and looked at the docs.	c2b6a24dac4832e5b7a0f322948ce64ac2c83e8b
	Add a management command to clean up orphaned data. This change adds a new management command that will clean up various pieces of data in the database and filesystem that could have been left behind if administrators used the "Delete Permanently" action on a review request or deleted review requests via the admin UI. It will query each of the affected items to get a total count, and then perform the actual deletion in batches. This allows us to show nice progress bars, as well as make the command interruptible. Testing Done: - Populated my database with a bunch of data including screenshots, file attachments, diffs, and change descriptions. Temporarily disabled our new signal handler and deleted the review requests. Ran with --show-counts-only and saw the expected counts for each type of object. Ran normally and watched it delete all the objects. Ran again with --show-counts-only and saw that it no longer reported any orphaned data. - Set the batch size low and verified that the progress bars worked correctly. - Built and looked at the docs.	e45b6c7f2d5c4d59c0c67d381f2cbf62dffef994

Diff:

Revision 4 (+690)

Show changes

	docs/manual/admin/sites/management-commands.rst
	reviewboard/dependencies.py
	reviewboard/reviews/management/commands/clean-orphaned-data.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Status:: Completed
Change Summary:: Pushed to release-7.x (e2b13e6)