RehanAhmad25 commented on issue #69543:
URL: https://github.com/apache/airflow/issues/69543#issuecomment-5278983762

   > We have two Ps [#69925](https://github.com/apache/airflow/pull/69925) and 
[#69746](https://github.com/apache/airflow/pull/69746) that fix this in 
different ways. Please discuss the soluion in this issue first before 
continuing further work.
   
   Since two PRs (#69746 and #69925) are fixing this differently, summarizing 
both here so the approach can be decided in one place.
   
   **#69746 (TimeOfDayTrigger)** keeps `start_from_trigger` working. It found 
and fixed an additional real bug along the way: `start_trigger_args` is a 
class-level attribute, and the original code mutated it directly, so multiple 
`TimeSensor` instances could end up sharing the same (wrong) moment. The fix 
stores parse-stable data and resolves the concrete UTC moment lazily when the 
trigger starts. Cost: a new trigger class plus DST edge-case handling 
(spring-forward gaps, fall-back ambiguity).
   
   **#69925** deprecates `start_from_trigger` for `TimeSensor` entirely: it's 
now a property that always reads `False`, with a setter that emits a 
deprecation warning on any assignment (at construction or afterward). 
`target_datetime` becomes a recomputed property instead of a stale parse-time 
value. Simpler diff, but drops a working, documented capability, anyone 
currently relying on `start_from_trigger=True` to skip a worker slot loses that 
on upgrade.
   
   Both fix the churn. The real question is whether the worker-slot 
optimization `start_from_trigger` provides is worth the added complexity of 
lazy resolution + DST handling to preserve, versus removing it outright. Would 
appreciate maintainer input on which direction to commit to so we're not 
duplicating review effort across two PRs.


-- 
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]

Reply via email to