Vamsi-klu commented on issue #69543: URL: https://github.com/apache/airflow/issues/69543#issuecomment-5463684321
The two approaches asked about in https://github.com/apache/airflow/issues/69543#issuecomment-5259850995 have now been discussed here. I think we can pick one. Keep and fix start_from_trigger (https://github.com/apache/airflow/pull/69746). Do not drop it (https://github.com/apache/airflow/pull/69925, also https://github.com/apache/airflow/pull/69610). What the bug actually is: TimeSensor.__init__ runs at parse time, combines datetime.now() with target_time, and with start_from_trigger=True writes that absolute moment into serialized start_trigger_args. The hash changes every parse. Main already isolates start_trigger_args per instance via dataclasses.replace. The remaining defect is the volatile moment. #69746 implements the lazy-moment design @potiuk asked for on that PR: TimeOfDayTrigger serializes only parse-stable target_time + tz + end_from_trigger, and resolves the UTC moment when the trigger starts. serialize() then persists moment so a reconstruct after midnight does not move the wait. start_from_trigger stays in the public signature. No major bump of apache-airflow-providers-standard. Docs are corrected: wall-clock date in the Dag timezone, not data_interval_end/run_after. #69925 (and #69610) stop the feature: ValueError originally, then a property that always reads False and warns. That is a functional regression for anyone using start_from_trigger=True to skip a worker slot. @potiuk called that out as needing a major provider bump if we went that way. @uranusjr later wrote here that dropping it is suboptimal and that we should try to fix the feature first (https://github.com/apache/airflow/issues/69543#issuecomment-5312368669). @RehanAhmad25 then said he would open a new lazy-resolution PR instead of continuing #69925; that new PR is not up, and #69925 still deprecates the flag. #69746 has been reworked since the Aug 11 pause (TimeOfDayTrigger, per-instance StartTriggerArgs, hash-stability tests with two frozen clocks, DST spring/fall, signature kept). @potiuk dismissed his older CHANGES_REQUESTED today to unblock. The remaining GitHub block is the Aug 11 CHANGES_REQUESTED on both PRs that asked us to decide here. Proposal: take #69746 as the fix, close #69925 and #69610 as superseded, and leave start_from_trigger working. Happy to adjust DST policy if maintainers want a different one. Not proposing to drop the API. --- Drafted-by: Cursor Grok 4.6; reviewed by @Vamsi-klu before posting -- 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]
