Fix TimeSensor start_from_trigger behavior

[RehanAhmad25]

Closes: #69543

What does this PR do?

TimeSensor computed its target datetime in __init__, which runs at DAG parse time using datetime.now(self.dag.timezone). When start_from_trigger=True, this value was stored directly in start_trigger_args.trigger_kwargs, which is serialized as part of the DAG. Because the value changed on every parse, the serialized DAG hash also changed on every parse, continuously creating new DAG versions.

This PR:

• Raises a ValueError when start_from_trigger=True is used with TimeSensor, since the target moment cannot be safely computed at parse time. deferrable=True remains fully supported and unaffected.
• Changes target_datetime into a property that is computed on access instead of being fixed during __init__.
• Updates the TimeSensor documentation to describe the actual behavior, which uses the current wall-clock time in the DAG's timezone, instead of the previously documented data_interval_end/run_after-relative behavior that was never implemented.

Was generative AI tooling used to co-author this PR?

• Yes (Claude)

Generated-by: Claude (Anthropic) following the guidelines

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

• Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
• Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
• Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@airflow.apache.org
  Slack: https://s.apache.org/airflow-slack

[aaron-y-chen]

Thanks for the PR :)

[aaron-y-chen]

I'm afraid this could cause a backward compatibility issue. Perhaps we should not raise an error directly in the initialization phase.

[RehanAhmad25]

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.

[uranusjr]

This change doesn’t seem to be meaningful? (Unless it’s doing a race condition)

I’d revert it.

[RehanAhmad25]

This one's intentional, your parenthetical guess is right. target_datetime is now a computed property (calls datetime.now() fresh on every access), so calling self.target_datetime twice in this method could return two slightly different instants if the wall clock ticks past the target moment between the log call and the comparison. Storing it in a local variable once guarantees the logged value and the compared value are the same instant. I'd like to keep this as-is rather than revert, let me know if you still see it differently.

[uranusjr]

Can this attribute be removed if it’s always False? And can it be set to True?

[RehanAhmad25]

No, it can't be set to True anymore, it'll always be False now. With the switch to a deprecation warning instead of a hard error, passing True just triggers the warning and silently falls back to False. Keeping the attribute itself around for backward compatibility, since TimeSensorAsync in this same file already does the same thing (deprecated flag kept as an inert attribute instead of being removed outright).

[uranusjr]

Keeping the value is a good idea, but I don’t like it being an attribute since the user would try to change it and see it’s silently ignored. I would change it to a property instead (with a deprecation warning) so setting it emits an error.

[RehanAhmad25]

Good point, updated it: start_from_trigger is now a property with a setter instead of a plain attribute. Reading it still always returns False, but setting it (whether at construction or afterward, e.g. op.start_from_trigger = True) now emits the same deprecation warning each time rather than only warning once and then silently ignoring further attempts. Tests updated to cover both the constructor path and direct assignment after construction.

[uranusjr]

This and #69746 fix the same thing in different ways. We should first figure out the correct approach in the issue.

[RehanAhmad25]

This and #69746 fix the same thing in different ways. We should first figure out the correct approach in the issue.

Agreed, let's settle this on the issue so it's not split across two threads.

For what it's worth, my read on the tradeoff: #69746's TimeOfDayTrigger genuinely fixes more than mine does, it caught and fixed the class-level start_trigger_args mutation bug independently, which mine only sidesteps (the buggy code path becomes dead code once start_from_trigger is deprecated, but I didn't find that bug myself). That's a real point in its favor.

My case for the simpler deprecation approach: start_from_trigger avoids one worker-slot poke/defer cycle by starting the sensor directly on the triggerer, a fairly narrow optimization. Preserving it costs a new trigger class, lazy-resolution logic, and DST edge case handling (spring-forward gaps, fall-back ambiguity) that's now permanent surface area to maintain and could itself grow bugs over time. Given how narrow the benefit is, I think removing it is proportionate, but I'm not tied to that if the consensus lands the other way, happy to adapt or step back from this fix if #69746 is the preferred direction.

For reference, the current state on this PR: start_from_trigger is now a property (always reads False) with a setter that emits a deprecation warning on any assignment, at construction or afterward, so nothing silently gets ignored without a signal. I'll post this same comparison on the issue.