Skip to content

Conversation

@SameerMesiah97
Copy link
Contributor

@SameerMesiah97 SameerMesiah97 commented Dec 27, 2025

Description

A defensive (mode = "before") Pydantic validator has been added to DagRunContext to safely handle detached ORM DagRun objects. The validator reloads the DagRun object from the DB with consumed_asset_events when relationship access would otherwise raise DetachedInstanceError, preventing scheduler crashes in DagRun timeout and callback paths. This complements existing eager-loading fixes added in PR #56916 and PR #59714.

Rationale

DagRunContext may be constructed with ORM DagRun instances that are no longer session-bound (e.g. during scheduler timeout handling). Accessing lazily loaded relationships such as consumed_asset_events on detached instances can raise DetachedInstanceError, which currently propagates and causes the scheduler to exit unexpectedly.

This issue has surfaced multiple times in different DagRun code paths. Centralizing a defensive guard in DagRunContext ensures that missing or detached relationship data does not crash the scheduler. Using a session to reload the DagRun within the validator is an acceptable tradeoff to maintain correctness with regards to the state of consumed_asset_events. Also, it is assumed that the reloaded DagRun entity must exist in the DB for code paths where DagRunContext is called.

Note that this validator only applies to ORM DagRun objects and not fastapi data models.

Tests

  • Added a test covering DagRunContext construction with a detached ORM DagRun, asserting that consumed_asset_events is present afterwards.
  • Added a test covering the attached ORM DagRun path to ensure existing behavior is preserved.
  • Existing DagRunContext tests for non-ORM/datamodel inputs remain unchanged and continue to pass.

Closes: #59740

@SameerMesiah97 SameerMesiah97 force-pushed the 59740-DagRunContext-Validation branch from 8dcf9a4 to 095248d Compare December 27, 2025 23:42
@Lee-W Lee-W self-requested a review December 29, 2025 10:21
@SameerMesiah97 SameerMesiah97 force-pushed the 59740-DagRunContext-Validation branch 4 times, most recently from be3315c to fac79a3 Compare January 7, 2026 20:30
@SameerMesiah97 SameerMesiah97 force-pushed the 59740-DagRunContext-Validation branch from efc8cd9 to 796b081 Compare January 7, 2026 21:23
@SameerMesiah97
Copy link
Contributor Author

@Lee-W @uranusjr

Sorry for tagging you here. But I just wanted to know if any more action is required from me on this.

Also, I just wanted to know if this is ready for merge or if it requires further review.

@Lee-W
Copy link
Member

Lee-W commented Jan 18, 2026

Nothing is needed on your end. I just want to wait for a second pair of eyes :) let's wait for a bit longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gracefully Handle Invalid consumed_asset_event in DagRunContext

3 participants