Fix DateTimeSensorAsync crashing Dag parsing with templated target_time#70310
Fix DateTimeSensorAsync crashing Dag parsing with templated target_time#70310haseebmalik18 wants to merge 1 commit into
Conversation
| :param target_time: datetime after which the job succeeds. (templated) | ||
| :param start_from_trigger: Start the task directly from the triggerer without going into the worker. | ||
| This requires a static ``target_time`` (a datetime or ISO-8601 string). A templated | ||
| ``target_time`` is not supported here because the trigger is created at Dag-parse time, |
There was a problem hiding this comment.
The trigger is not created at Dag-parse time, but either in the worker or the triggerer
There was a problem hiding this comment.
Yes you're right, I overlooked that. Will reword the docstring.
| 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, |
There was a problem hiding this comment.
How does it solve the crashing? It seems to improve the error message rather than fix the issue.
There was a problem hiding this comment.
@shahar1 - I think the goal was to replicate something like this: https://github.com/apache/airflow/pull/69610/changes
There was a problem hiding this 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:
- 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.
- On Airflow 3.3+, pass the raw target_time string through trigger_kwargs and let the triggerer render it before the trigger runs. Since Re-enable start_from_trigger feature with rendering of template fields #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.
3 participants
Fixes
DateTimeSensorAsynccrashing Dag parsing whentarget_timeis a Jinja template andstart_from_trigger=True.With
start_from_trigger=True,target_timegets parsed in__init__at Dag-parse time, before Jinja renders it. So passing a template (which is the field's documented use) blew up with apendulumParserErrorand took down the whole Dag file. Now a statictarget_timestill works, and a templated one raises a clearValueErrorinstead.closes: #70284