nanjeshramesh opened a new pull request, #70576:
URL: https://github.com/apache/airflow/pull/70576
`DateTimeSensorAsync.__init__` calls `timezone.parse(self.target_time)` at
Dag-parse time
when `start_from_trigger=True`, before Jinja rendering has happened.
`target_time` is a
documented template field (e.g. `"{{
data_interval_end.tomorrow().replace(hour=1) }}"`), so
passing a template — the field's normal, documented use case — raises a raw
`pendulum.parsing.exceptions.ParserError` right there, crashing parsing of
the **entire Dag
file**, not just the one task.
## Relation to #70310 and #70469
Both prior attempts at this issue (#70310 by @haseebmalik18, #70469 by
@burakcoleman) catch
the parse failure in `__init__` and raise a clearer `ValueError` instead of
the raw
`ParserError`. That's a real improvement to the error message, but as
reviewers on both PRs
pointed out, it doesn't fix the crash — the Dag still fails to parse whenever
`start_from_trigger=True` and `target_time` is a template.
This PR takes the approach @haseebmalik18 outlined in the #70310 review
discussion: defer
resolving `target_time` until the triggerer can render it, the same way
`FileSensor` already
handles a templated `filepath`.
## What this PR does
On Airflow >= 3.3, instead of parsing `target_time` at Dag-parse time, the
raw (unrendered)
template string is passed straight through to `DateTimeTrigger` via
`start_trigger_args.trigger_kwargs` under the key `"target_time"` — which
matches
`DateTimeSensor.template_fields`. `BaseTrigger`'s `task_instance` setter
already picks up any
`trigger_kwargs` key that matches an operator template field (see
`airflow.triggers.base.BaseTrigger`), and `TriggererJobRunner.run_trigger`
calls
`render_template_fields()` on the trigger before invoking `run()` — this is
exactly the
mechanism introduced in #55068 and that `FileSensor` already relies on for
`filepath`.
`DateTimeTrigger` now accepts either a resolved `moment` (unchanged,
existing behavior) or a
raw `target_time` string. In the latter case, `moment` starts as `None` and
is lazily parsed
from `target_time` the first time it's needed (`run()` or `serialize()`), by
which point the
triggerer has already rendered the template in place.
On Airflow < 3.3, that triggerer-side rendering mechanism doesn't exist yet,
so a templated
`target_time` can never be resolved via `start_from_trigger`. Rather than
crash Dag parsing on
every parse cycle, `start_from_trigger` is disabled with a `log.warning`,
and the task falls
back to deferring from the worker via `execute()` instead — at that point
`target_time` has
already been rendered normally by the scheduler/worker, so it works
correctly, just without
the trigger-only optimization.
Static/ISO `target_time` values are completely unaffected on any version.
## Testing
- Added unit tests in `test_temporal.py` covering: `DateTimeTrigger`
accepting an unrendered
`target_time` at construction without raising, resolving it once rendered,
raising a clear
error if it's still unrendered when needed, rejecting both/neither of
`moment`/`target_time`,
and a full simulated triggerer flow (construct with raw template →
`render_template_fields`
→ `run()`) that fires at the correct time.
- Added unit tests in `test_date_time.py` covering
`DateTimeSensorAsync.__init__` branching on
`AIRFLOW_V_3_3_PLUS` for a templated `target_time`, and confirming a
static `target_time`
behaves identically regardless of that flag.
- Ran the exact reproduction script from #70284: confirmed it raises
`pendulum.parsing.exceptions.ParserError` on `main`, and parses cleanly
with this fix.
- Full existing test suites for both files pass (32/32), no regressions.
- `ruff check` / `ruff format --diff` clean on all changed files.
closes: #70284
Related: #70310, #70469, #55068, #69610
##### Was generative AI tooling used to co-author this PR?
- No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
