haseebmalik18 commented on code in PR #70310:
URL: https://github.com/apache/airflow/pull/70310#discussion_r3647353860
##########
providers/standard/src/airflow/providers/standard/sensors/date_time.py:
##########
@@ -125,8 +128,14 @@ def __init__(
self.start_from_trigger = start_from_trigger
if self.start_from_trigger:
+ try:
+ moment = timezone.parse(self.target_time)
+ except ValueError as e:
+ raise ValueError(
+ f"start_from_trigger=True requires a static target_time,
not a template: {self.target_time!r}"
+ ) from e
self.start_trigger_args.trigger_kwargs = dict(
- moment=timezone.parse(self.target_time),
+ moment=moment,
Review Comment:
Fair point. As is, this just turns a cryptic ParserError into a clearer
ValueError, the Dag file still fails to parse. And yes, the intent was to
mirror #69610
The difference is that for TimeSensor, start_from_trigger could never work,
since it computes "today at target_time" from the wall clock at parse time.
Here target_time is a template field, so there are I think two ways to go about
it:
1. If target_time is not parseable at parse time, log a warning and fall
back to deferring from the worker. The template renders as usual and the Dag
parses fine on all supported versions.
2. On Airflow 3.3+, pass the raw target_time string through trigger_kwargs
and let the triggerer render it before the trigger runs. Since #55068 the
triggerer renders any trigger kwarg whose name matches an operator template
field, which FileSensor already relies on for filepath. That would make
start_from_trigger actually work with a templated target_time, but it means
extending DateTimeTrigger to accept a target_time kwarg and gating on the
Airflow version, falling back to (1) on older versions.
--
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]
