[v3-2-test] Preload source_aliases in process_executor_events (#65422)#66191
Merged
[v3-2-test] Preload source_aliases in process_executor_events (#65422)#66191
Conversation
PR #56916 added a _eager_load_dag_run_for_validation helper that returns two loader options so DRDataModel.model_validate can walk DagRun.consumed_asset_events[*].asset and source_aliases without triggering lazy loads on a detached ORM instance. The helper's docstring example applies both loaders, and _adopt_or_reset_orphaned_tasks applies both. SchedulerJobRunner.process_executor_events only applies asset_loader and discards alias_loader, leaving AssetEvent.source_aliases lazy-loaded. When Celery/Kubernetes executor reports a failed-while-queued event for an asset-triggered DagRun whose consumed AssetEvent has a non-empty source_aliases collection, SQLAlchemy detaches the AssetEvent by the time pydantic walks the DagRun, and source_aliases raises DetachedInstanceError wrapped as a pydantic ValidationError. The exception escapes _run_scheduler_loop, the scheduler exits, which orphans in-flight task instances. The next scheduler's _adopt_or_reset_orphaned_tasks picks them up and replays the same validation path, crashing again. Apply alias_loader alongside asset_loader at this call site. Extend test_process_executor_events_with_asset_events so the AssetEvent now carries a non-empty source_aliases collection and the assertion verifies the alias survives into the callback payload. (cherry picked from commit ef0ff5c) Co-authored-by: Pablo Seibelt <pabloseibelt@sicarul.com> Co-authored-by: Claude <noreply@anthropic.com>
1 task
boring-cyborg
Bot
added
the
area:Scheduler
Lee-W
approved these changes
May 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR #56916 added a _eager_load_dag_run_for_validation helper that
returns two loader options so DRDataModel.model_validate can walk
DagRun.consumed_asset_events[*].asset and source_aliases without
triggering lazy loads on a detached ORM instance. The helper's
docstring example applies both loaders, and _adopt_or_reset_orphaned_tasks
applies both. SchedulerJobRunner.process_executor_events only applies
asset_loader and discards alias_loader, leaving
AssetEvent.source_aliases lazy-loaded.
When Celery/Kubernetes executor reports a failed-while-queued event
for an asset-triggered DagRun whose consumed AssetEvent has a
non-empty source_aliases collection, SQLAlchemy detaches the
AssetEvent by the time pydantic walks the DagRun, and source_aliases
raises DetachedInstanceError wrapped as a pydantic ValidationError.
The exception escapes _run_scheduler_loop, the scheduler exits,
which orphans in-flight task instances. The next scheduler's
_adopt_or_reset_orphaned_tasks picks them up and replays the same
validation path, crashing again.
Apply alias_loader alongside asset_loader at this call site.
Extend test_process_executor_events_with_asset_events so the
AssetEvent now carries a non-empty source_aliases collection and
the assertion verifies the alias survives into the callback payload.
(cherry picked from commit ef0ff5c)
Co-authored-by: Pablo Seibelt pabloseibelt@sicarul.com
Co-authored-by: Claude noreply@anthropic.com