Skip to content

chore(workflow-engine): Modify cache utils to fix failing tests - #120747

Open
leeandher wants to merge 4 commits into
leanderrodrigues/iswf-3123-typing-fixes-before-migrationfrom
leanderrodrigues/iswf-3123-test-fixes-for-migration
Open

chore(workflow-engine): Modify cache utils to fix failing tests #120747
leeandher wants to merge 4 commits into
leanderrodrigues/iswf-3123-typing-fixes-before-migrationfrom
leanderrodrigues/iswf-3123-test-fixes-for-migration

Conversation

@leeandher

Copy link
Copy Markdown
Member

The cascade deletions cause issues with these caching functions. The test failures can be seen in the migration PR. I'm not too familiar with these utilities but from my understanding:

  • It's causing failures because the cascade deletion order has changed
  • with the NOT NULL key to Project: the Detector was always deleted before the Project, meaning Detector's post_delete fires, and that Project is still in the DB, so detector.project.organization (on the in-memory Detector) still works.
  • with the nulllable FK to Project: Django does not ensure that Detector's post_delete happens before the Project row is removed. When the Project is removed first, the caching function hits the DoesNotExist error.

I consulted claude about this ordering change, who added:

the reason the ordering changes is specific to how Django's cascade collector works. It tracks dependencies between models to determine deletion order, but it only registers a dependency when the FK is NOT NULL (line 136 of django/db/models/deletion.py: if source is not None and not nullable). And the CASCADE handler passes nullable=field.null (line 27). So making the FK nullable literally removes the ordering constraint that kept Detector-before-Project, and the deletion order becomes arbitrary.

Previous PR: #120744
Next PR: #120687

@linear-code

linear-code Bot commented Jul 28, 2026

Copy link
Copy Markdown

ISWF-3123

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 28, 2026
@leeandher
leeandher force-pushed the leanderrodrigues/iswf-3123-test-fixes-for-migration branch from 43104fc to 196a710 Compare July 28, 2026 17:04
@sentry

sentry Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Sentry Snapshot Testing

Name Added Removed Changed Renamed Unchanged Skipped Status
sentry-frontend
sentry-frontend
0 0 0 0 451 0 ✅ Unchanged

⚙️ sentry-frontend Snapshot Settings

@leeandher
leeandher force-pushed the leanderrodrigues/iswf-3123-test-fixes-for-migration branch from 196a710 to aef80fb Compare July 28, 2026 18:01
@github-actions

This comment was marked as outdated.

@leeandher
leeandher force-pushed the leanderrodrigues/iswf-3123-test-fixes-for-migration branch from aef80fb to 081ffb5 Compare July 28, 2026 18:11
@leeandher
leeandher marked this pull request as ready for review July 28, 2026 18:40
@leeandher
leeandher requested review from a team as code owners July 28, 2026 18:40
Comment thread src/sentry/utils/function_cache.py

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 081ffb5. Configure here.

Comment thread src/sentry/uptime/models.py
Comment thread src/sentry/uptime/models.py
@leeandher
leeandher requested a review from a team July 28, 2026 21:05

def get_org_from_detector(detector: Detector) -> tuple[Organization]:
return (detector.linked_project.organization,)
def get_org_from_detector(detector: Detector) -> tuple[Organization] | None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rather than this returning none, can we raise an exception? My thought here is, I would expect every detector to belong to an org, and if it isn't then it should be treated as an error.

That would also mean that the function_cache wouldn't need to change 🎉 we'd just call into here, raise an unhandled exception, and catch that with sentry.

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.

2 participants