RehanAhmad25 commented on code in PR #69925:
URL: https://github.com/apache/airflow/pull/69925#discussion_r3652289102
##########
providers/standard/src/airflow/providers/standard/sensors/time.py:
##########
@@ -62,34 +62,36 @@ def __init__(
trigger_kwargs: dict[str, Any] | None = None,
**kwargs,
) -> None:
+ if start_from_trigger:
+ raise ValueError(
+ "TimeSensor does not support start_from_trigger=True. The
target moment is "
+ "computed fresh from the current wall-clock time on every Dag
parse, so baking "
+ "it into the serialized trigger arguments makes the serialized
Dag hash change "
+ "on every parse. Use deferrable=True instead, which computes
the target moment "
+ "at task execution time and does not have this problem."
+ )
Review Comment:
Good point, you're right, a hard error at parse time is too risky here.
Updated it: `start_from_trigger=True` now just emits a deprecation warning and
gets ignored (falls back to `False`), instead of raising. So existing DAGs
using it won't break on upgrade, they'll just see a warning telling them to
switch to `deferrable=True`. Updated the test and docs to match. Let me know if
that works better.
--
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]