Skip to content

Load the correct DAG version when a task starts from a trigger#69988

Open
seanghaeli wants to merge 1 commit into
apache:mainfrom
aws-mwaa:fix/triggerer-pin-dag-version
Open

Load the correct DAG version when a task starts from a trigger#69988
seanghaeli wants to merge 1 commit into
apache:mainfrom
aws-mwaa:fix/triggerer-pin-dag-version

Conversation

@seanghaeli

@seanghaeli seanghaeli commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

If the run and the task point at different DAG versions, the trigger should load the run's version.

@seanghaeli
seanghaeli force-pushed the fix/triggerer-pin-dag-version branch 4 times, most recently from 5e55a88 to f157049 Compare July 17, 2026 00:52
@seanghaeli
seanghaeli force-pushed the fix/triggerer-pin-dag-version branch from f157049 to ecbf158 Compare July 17, 2026 00:54
@seanghaeli
seanghaeli marked this pull request as ready for review July 17, 2026 00:59
@seanghaeli seanghaeli changed the title Pin triggerer start_from_trigger DAG resolution to the dagrun's version Load the correct DAG version when a task starts from a trigger Jul 17, 2026
@seanghaeli

Copy link
Copy Markdown
Contributor Author

@o-nikolas this one is similar in spirit to your PR #69941

@o-nikolas o-nikolas 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.

Seems like a reasonable change semantically. But someone who knows the trigger code better should review to see if this is what we want (it seems like it should be though to me at least). Maybe @vincbeck?

@viiccwen viiccwen 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.

left comments.



def test_create_workload_resolves_serialized_dag_from_run_created_version(jobless_supervisor, mocker):
"""If the run and the task point at different DAG versions, the trigger should load the run's version."""

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.

It should be "Dag", not "DAG".

Comment on lines +879 to +880
version_id=trigger.task_instance.get_dagrun(session=session).created_dag_version_id
or trigger.task_instance.dag_version_id,

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.

created_dag_version_id is populated even for Dags with disable_bundle_versioning=True; dag_run.bundle_version is what determines
whether the run is pinned. This unconditional preference for created_dag_version_id therefore makes an unpinned start_from_trigger task load the Dag version from when the run was created, even after the scheduler has advanced the unfinished TI's dag_version_id following a reparse.

This differs from DBDagBag._version_from_dag_run(), which intentionally resolves the latest version when bundle_version is absent. Please use the run's created_dag_version_id only when the run is pinned, retain the TI version for unpinned runs, and cover both cases in the regression test.

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.

Please expand the test to cover both pinned and unpinned cases and set dag_run.bundle_version explicitly.
Also, please add spec/autospec to the new mocks and remove assert run_version != umped_ti_version; that assertion tests uuid.uuid4() rather than behavior introduced by this PR.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants