-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Fix rendering of template fields with start from trigger #53071
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix rendering of template fields with start from trigger #53071
Conversation
…can be part of the AbstractOperator and moved rendering logic from expand_start_trigger_args to _do_render_template_fields so that all the rendering logic is common in on method
…ask method of TaskInstance
…xpand_start_trigger_args method of BaseOperator
# Conflicts: # airflow-core/src/airflow/models/baseoperator.py # airflow-core/src/airflow/models/mappedoperator.py # airflow-core/src/airflow/serialization/serialized_objects.py # airflow-core/tests/unit/models/test_baseoperator.py # airflow-core/tests/unit/models/test_dagrun.py # airflow-core/tests/unit/serialization/test_dag_serialization.py
|
@dabla can someone else take forward your work by resolving conflicts and conversations as they come |
|
Tests fail |
@eladkal Test are fixed |
|
Moving this to 3.1 as it's never worked, so this is a new feature, not a bug fix |
uranusjr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if main would work if I merge this after #54646. Merging main in to make sure first.
This reverts commit 4362c25 from apache#53071. The original change violated Airflow's architectural separation by making the Triggerer supervisor process load DAGs and attempt template rendering. This breaks with mapped operators that use lazy sequences requiring SUPERVISOR_COMMS, which only exists in task execution context. The Triggerer should operate purely on trigger database records and pre-computed trigger kwargs, not attempt runtime DAG operations. Fixes triggerer failures with mapped operators using XCom dependencies.
…5037) This reverts commit 4362c25 from #53071. The original change violated Airflow's architectural separation by making the Triggerer supervisor process load DAGs and attempt template rendering. This breaks with mapped operators that use lazy sequences requiring SUPERVISOR_COMMS, which only exists in task execution context. The Triggerer should operate purely on trigger database records and pre-computed trigger kwargs, not attempt runtime DAG operations. Fixes triggerer failures with mapped operators using XCom dependencies.
) * refactor: Render templated fields before starting from trigger * refactor: Moved StartTriggerArgs from airflow-core to task-sdk so it can be part of the AbstractOperator and moved rendering logic from expand_start_trigger_args to _do_render_template_fields so that all the rendering logic is common in on method * refactor: Import missing datetime * refactor: Fixed import of datetime * refactor: Reformatted files * refactor: Changed import of StartTriggerArgs from new location in airflow-core * refactor: Moved call of render_template_fields from DagRun to defer_task method of TaskInstance * refactor: Moved call of render_template_fields from TaskInstance to expand_start_trigger_args method of BaseOperator * refactor: Do the template rendering in update_triggers of TriggerRunnerSupervisor * refactor: Fixed imports tests * refactor: Fixed imports * refactor: Reformatted update_triggers * refactor: Reformatted expand_start_trigger_args * refactor: Revert move of StartTriggerArgs to airflow-sdk * Revert "refactor: Revert move of StartTriggerArgs to airflow-sdk" This reverts commit 5918847. * refactor: Defined StartTriggerArgs as deprecated classes in old import location * refactor: Moved definition of StartTriggerArgs as deprecated classes in old import location to base instead of init * refactor: Moved import of StartTriggerArgs in type checking block * refactor: Added StartTriggerArgs in __all__ * Revert "refactor: Added StartTriggerArgs in __all__" This reverts commit 2e2721b. * Revert "refactor: Moved definition of StartTriggerArgs as deprecated classes in old import location to base instead of init" This reverts commit 50a8a91. * refactor: Import StartTriggerArgs in type checking block * refactor: Moved definition of StartTriggerArgs as deprecated classes in old import location to base instead of init * refactor: Fixed inner import of StartTriggerArgs in expand_start_trigger_args * refactor: Use explicit re-export for StartTriggerArgs * Revert "refactor: Moved definition of StartTriggerArgs as deprecated classes in old import location to base instead of init" This reverts commit 8a4e0c6. * Revert "refactor: Use explicit re-export for StartTriggerArgs" This reverts commit b14cb93. * Revert "refactor: Import StartTriggerArgs in type checking block" This reverts commit c1533ac. * refactor: Removed StartTriggerArgs from deprecated_classes and override getattr in base module instead * refactor: Updated warning message * refactor: Reformatted warning message * refactor: Reformatted base * refactor: Reformatted getattr to not do inner import to avoid static check error * refactor: Try mocking DagBag for test_trigger_lifecycle * refactor: Reorganized imports * refactor: Try to fix test_trigger_create_race_condition_38599 * refactor: Try to fix has not been assigned to a DAG yet error in test_trigger_create_race_condition_38599 * refactor: Try fixing DagRun validation errors for start_date and run_type in test_trigger_create_race_condition_38599 * refactor: expand_start_trigger_args method doesn't have session parameter anymore * refactor: Try fixing remaining tests * refactor: Reformatted test trigger job * refactor: Check if start_trigger_args is not None * refactor: Removed old imports * refactor: Fixed import issues * refactor: Reformatted test_trigger_create_race_condition_38599 * refactor: Try to fix test_update_triggers_prevents_duplicate_creation_queue_entries * refactor: Reformatted test_update_triggers_prevents_duplicate_creation_queue_entries * refactor: Added StartTriggerArgs to api.rst * refactor: Added StartTriggerArgs in __init__.pyi * refactor: Added lazy import for StartTriggerArgs * refactor: Changed order in __all__ for StartTriggerArgs * refactor: Reorganized imports for MappedOperator --------- Co-authored-by: David Blain <david.blain@infrabel.be> Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
…ache#55037) This reverts commit 4362c25 from apache#53071. The original change violated Airflow's architectural separation by making the Triggerer supervisor process load DAGs and attempt template rendering. This breaks with mapped operators that use lazy sequences requiring SUPERVISOR_COMMS, which only exists in task execution context. The Triggerer should operate purely on trigger database records and pre-computed trigger kwargs, not attempt runtime DAG operations. Fixes triggerer failures with mapped operators using XCom dependencies.
…ache#55037) This reverts commit 4362c25 from apache#53071. The original change violated Airflow's architectural separation by making the Triggerer supervisor process load DAGs and attempt template rendering. This breaks with mapped operators that use lazy sequences requiring SUPERVISOR_COMMS, which only exists in task execution context. The Triggerer should operate purely on trigger database records and pre-computed trigger kwargs, not attempt runtime DAG operations. Fixes triggerer failures with mapped operators using XCom dependencies.
…5037) This reverts commit 4362c25e5cb96e3b6c99497b682d4dfe23a86aba from apache/airflow#53071. The original change violated Airflow's architectural separation by making the Triggerer supervisor process load DAGs and attempt template rendering. This breaks with mapped operators that use lazy sequences requiring SUPERVISOR_COMMS, which only exists in task execution context. The Triggerer should operate purely on trigger database records and pre-computed trigger kwargs, not attempt runtime DAG operations. Fixes triggerer failures with mapped operators using XCom dependencies. GitOrigin-RevId: 5d1d9567a24d3ff2b20eb153627d11e7d73fa7b3
This PR fixes the issue described in #53063 in which template fields aren't rendered before being assigned to the trigger. This is because before the start_from_trigger mechanism existed, a deferred operator would always run in the worker first, which by default, would have rendered the templated fields before deferring the task.
closes: #53063
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.