Repository navigation
Avoid parsing templated DateTimeSensorAsync targets at Dag parse time - #72659
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
Prevent templated targets from crashing Dag parsing when triggerer-start is requested. The value is rendered only after parsing, so it must be resolved before the trigger is constructed.
…er args A Dag author who asks for start_from_trigger=True with a templated target_time now silently gets the worker path, so say so on the parameter. The extra assertion pins the other half of the guard: the class-level start_trigger_args must be left alone rather than replaced with a copy built from the raw template. Generated-by: Claude Opus 5
319c554 to
24ce45d
Compare
potiuk
left a comment
There was a problem hiding this comment.
The guard correctly stops DateTimeSensorAsync.__init__ from parsing a Jinja target_time at Dag-parse time and falls back to the worker path, where execute() resolves the rendered datetime before constructing DateTimeTrigger. That is in line with the earlier decision that the triggerer must not render templates itself, and the regression test does fail without the fix.
I pushed a small fixup on top and rebased the branch on main:
:param start_from_trigger:now says it is ignored whentarget_timeis a Jinja template. Without that, a Dag author who asked for triggerer-start has no way to find out why their task went through a worker.- The test also asserts
op.start_trigger_args is DateTimeSensorAsync.start_trigger_args, pinning the other half of the guard — the class attribute must be left alone rather than replaced with a copy built from the raw template. That complementstest_start_trigger_args_are_not_shared_between_tasks.
Smaller observations
- The hard-coded
{{/{%/{#check will not recognise custom delimiters set throughDAG(jinja_environment_kwargs=...). That matches how core already detects templates inairflow.utils.helpers.parse_template_string, so it is fine as is — just noting the parse-time crash still exists for that rare configuration. - Mapped tasks built with
partial(start_from_trigger=True).expand(target_time=[...])skip__init__, so the guard does not run on that path. That is pre-existing and documented, not something this PR regresses. - The PR description does not include the template's "Was generative AI tooling used to co-author this PR?" section. Could you restore it and answer it either way? See contributing-docs/05_pull_requests.rst.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.More on how Apache Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
Why
DateTimeSensorAsynccan be configured withstart_from_trigger=True, but templatedtarget_timevalues are rendered after Dag parsing. Calling_momentin__init__therefore parses the raw Jinja expression and crashes the Dag.What changed
Keep triggerer-start for concrete targets, but fall back to the normal worker path when
target_timecontains a Jinja template. The worker path resolves the rendered datetime before constructingDateTimeTrigger.Fixes #70284
Verification
uv run --project providers/standard pytest providers/standard/tests/unit/standard/sensors/test_date_time.py -xvs(16 passed)uvx prek run --stage pre-commit --files providers/standard/src/airflow/providers/standard/sensors/date_time.py providers/standard/tests/unit/standard/sensors/test_date_time.py