Skip to content

fix(difs): Use PROTECT for shared debug files#110411

Merged
szokeasaurusrex merged 1 commit intomasterfrom
szokeasaurusrex/fix/shared-file-delete-safety
Mar 16, 2026
Merged

fix(difs): Use PROTECT for shared debug files#110411
szokeasaurusrex merged 1 commit intomasterfrom
szokeasaurusrex/fix/shared-file-delete-safety

Conversation

@szokeasaurusrex
Copy link
Member

@szokeasaurusrex szokeasaurusrex commented Mar 11, 2026

ProjectDebugFile.file can now be shared by multiple rows.

Update the Django relation from CASCADE to PROTECT so ORM deletes match the intended ownership model. Keep deleting the backing File as a best-effort cleanup step after deleting a ProjectDebugFile, but leave it in place when another ProjectDebugFile still references it.

This prepares us for #106947, where the solution will be to allow multiple ProjectDebugFiles to share a single File row.

The database should already reject deleting a referenced File; this change makes Django raise ProtectedError instead of relying on a DB-level failure path.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 11, 2026
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/1051_protect_projectdebugfile_file.py

for 1051_protect_projectdebugfile_file in sentry

--
-- Alter field file on projectdebugfile
--
-- (no-op)

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/fix/shared-file-delete-safety branch from 5d53727 to 4cdc2ba Compare March 11, 2026 13:34
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/fix/shared-file-delete-safety branch from 4cdc2ba to cc5376b Compare March 11, 2026 14:13
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/fix/shared-file-delete-safety branch from cc5376b to 779eb11 Compare March 11, 2026 14:19
@szokeasaurusrex szokeasaurusrex changed the title fix: Disallow deleting files still referenced fix(difs): Use PROTECT for shared debug files Mar 11, 2026
@szokeasaurusrex szokeasaurusrex requested a review from jjbayer March 11, 2026 14:21
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/fix/shared-file-delete-safety branch 2 times, most recently from 5d53727 to f034483 Compare March 12, 2026 10:00
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/1052_protect_projectdebugfile_file.py

for 1052_protect_projectdebugfile_file in sentry

--
-- Alter field file on projectdebugfile
--
-- (no-op)

@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2026

Backend Test Failures

Failures on 32730b7 in this run:

tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_hourly_counts_matchlog
tests/sentry/issues/escalating/test_escalating.py:365: in test_eap_and_snuba_hourly_counts_match
    assert snuba_count == eap_count == 6
E   assert 0 == 6
tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_past_counts_match_multiple_groupslog
tests/sentry/issues/escalating/test_escalating.py:423: in test_eap_and_snuba_past_counts_match_multiple_groups
    assert eap_counts[group_a.id] == 2
E   KeyError: 233
tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_hourly_count_excludes_old_eventslog
tests/sentry/issues/escalating/test_escalating.py:382: in test_eap_hourly_count_excludes_old_events
    assert snuba_count == eap_count == 1
E   assert 0 == 1
tests/sentry/issues/escalating/test_escalating.py::TestEAPIsEscalating::test_eap_and_snuba_past_counts_match_single_grouplog
tests/sentry/issues/escalating/test_escalating.py:396: in test_eap_and_snuba_past_counts_match_single_group
    assert len(snuba_results) == len(eap_results) == 1
E   assert 0 == 1
E    +  where 0 = len([])

@szokeasaurusrex szokeasaurusrex marked this pull request as ready for review March 16, 2026 09:31
@szokeasaurusrex szokeasaurusrex requested a review from a team as a code owner March 16, 2026 09:31
`ProjectDebugFile.file` can now be shared by multiple rows.

Update the Django relation from `CASCADE` to `PROTECT` so ORM deletes match the intended ownership model. Keep deleting the backing `File` as a best-effort cleanup step after deleting a `ProjectDebugFile`, but leave it in place when another `ProjectDebugFile` still references it.

This prepares us for [#106947](#106947), where aliasing is a concrete use case for shared `File` rows.

The database should already reject deleting a referenced `File`; this change makes Django raise `ProtectedError` instead of relying on a DB-level failure path.
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/fix/shared-file-delete-safety branch from 2ec10ff to cc7ad3a Compare March 16, 2026 10:24
@szokeasaurusrex szokeasaurusrex enabled auto-merge (squash) March 16, 2026 10:26
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/1053_protect_projectdebugfile_file.py

for 1053_protect_projectdebugfile_file in sentry

--
-- Alter field file on projectdebugfile
--
-- (no-op)

@szokeasaurusrex szokeasaurusrex merged commit 9e8ecd4 into master Mar 16, 2026
78 of 79 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/fix/shared-file-delete-safety branch March 16, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants