Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
jason-ford-codecov
force-pushed
the
fix/database_deletes
branch
from
47048a2 to
8a8170b
Compare
| try: | ||
| cleaned_models = query._raw_delete(query.db) | ||
| except SoftTimeLimitExceeded: | ||
| log.warning( | ||
| "cleanup_queryset: soft time limit hit, rolling back delete of %s", | ||
| model.__name__, | ||
| exc_info=True, | ||
| ) | ||
| raise |
There was a problem hiding this comment.
Bug: The _raw_delete call can raise EmptyResultSet for empty querysets, but this exception is not caught, which will cause the cleanup task to fail unnecessarily.
Severity: MEDIUM
Suggested Fix
Wrap the _raw_delete(query) call in a try...except EmptyResultSet block. Inside the except block, you can simply pass or log that no rows were deleted, as this is an expected outcome for an empty queryset.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/worker/services/cleanup/cleanup.py#L46-L54
Potential issue: The `_raw_delete` function in `cleanup_queryset` is not wrapped in a
`try...except` block that handles `django.db.models.query.EmptyResultSet`. This
exception can be raised when processing an empty queryset, which is a normal scenario
when there are no records to delete. While the code correctly handles this exception
when generating a query string for logging, it fails to do so for the actual delete
operation. This inconsistency will cause the entire cleanup task to fail when it
encounters an empty set of records, treating a valid "0 rows deleted" case as an error.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #767 +/- ##
==========================================
- Coverage 92.26% 92.25% -0.01%
==========================================
Files 1304 1304
Lines 47921 47937 +16
Branches 1628 1628
==========================================
+ Hits 44216 44226 +10
- Misses 3396 3402 +6
Partials 309 309
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (64.70%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. 📢 Thoughts on this report? Let us know! |
1 participant
This adds more logging to the deletion process
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Note
Low Risk
Primarily adds logging and exception handling around existing
_raw_deletecalls; low functional risk, but log volume could increase during large cleanups.Overview
Adds detailed instrumentation around database deletions in
cleanup_queryset, logging the model name and a truncated SQL query before deletes and the affected row count after.Wraps
_raw_deleteto explicitly log and re-raiseSoftTimeLimitExceededandOperationalError(connection drops), and safely handlesEmptyResultSetwhen rendering the query string.Written by Cursor Bugbot for commit 8a8170b. This will update automatically on new commits. Configure here.