YAshhh29 opened a new pull request, #69534:
URL: https://github.com/apache/airflow/pull/69534
`OpenAIBatchTrigger` measures its polling timeout with `time.time()` (wall
clock) in two places, and `OpenAITriggerBatchOperator` sets the deadline
with `time.time() + self.timeout`. Because wall-clock time can jump — NTP
corrections, DST, container clock skew, VM pause/resume — a deferred batch
task can either time out early or run far past its intended deadline.
Airflow's own coding standards flag this exact pattern:
> `time.monotonic()` for durations, not `time.time()`.
>
> — `.github/instructions/code-review.instructions.md` and `AGENTS.md`
### Why the fix isn't a one-line swap
`time.monotonic()` values are only meaningful within a single process. The
operator (in the worker) and the trigger (in the Triggerer) run in
different processes, so the operator cannot pre-compute a monotonic
deadline and hand it to the trigger. The trigger must call
`time.monotonic()` itself.
To make that possible the trigger's preferred constructor argument
changed from `end_time` (absolute wall-clock deadline) to `timeout` (a
duration in seconds). The trigger now records `time.monotonic()` at the
start of `run()` and reports the elapsed monotonic duration on timeout.
### Backward compatibility
`OpenAIBatchTrigger.__init__` still accepts the legacy `end_time`
argument so that triggers serialised by the previous version of the
operator continue to run after an upgrade. When a legacy `end_time` is
present the trigger derives a best-effort remaining duration from the
wall clock once, then tracks the rest with the monotonic clock — so even
the legacy path is no longer fully at the mercy of wall-clock jumps
inside the polling loop. `serialize()` preserves whichever argument the
trigger was constructed with, so a rolling upgrade never rewrites an
in-flight trigger's schema.
### What changes
- `providers/openai/src/.../triggers/openai.py`
- Add `timeout: float | None` constructor arg and validation
(exactly one of `timeout`/`end_time` must be given).
- `run()` measures elapsed time via `time.monotonic()`.
- `serialize()` emits whichever field the trigger was constructed with.
- Timeout error message now reports actual elapsed seconds (previously
it reported `time.time() - self.end_time`, which is seconds *past*
the deadline — misleading and sometimes negative).
- `providers/openai/src/.../operators/openai.py`
- Passes `timeout=self.timeout` instead of computing `end_time`.
- Removes the now-unused `import time`.
- `providers/openai/tests/.../test_openai.py`
- Existing tests now use `timeout=`.
- New test `test_serialization_with_legacy_end_time` proves the
backward-compat serialisation path.
- New test `test_timeout_survives_wall_clock_jump_backward` — the
regression test — patches `time.time` to jump backward by an hour
while `time.monotonic` keeps ticking, and asserts the timeout still
fires on the monotonic clock. Fails against the pre-fix code.
- Constructor validation tests for the "neither" and "both" error paths.
- `providers/openai/docs/changelog.rst`: bug-fix note explaining the
behaviour change and the `end_time` → `timeout` migration.
### How to reproduce the original bug
1. Deploy a DAG with `OpenAITriggerBatchOperator(deferrable=True,
timeout=600)`.
2. While the batch is running and the trigger is deferred, force a
backward wall-clock adjustment on the Triggerer host (e.g.
`sudo date -s '10 minutes ago'`, or a large NTP step).
3. Observe that the trigger keeps polling well past 600 seconds because
`self.end_time < time.time()` is no longer true.
The same pattern also fires *early* on forward clock jumps.
### Related
- Airflow's own code-review rule:
`.github/instructions/code-review.instructions.md`
- Similar recent audit of ML-provider dep hygiene: #69408
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — GitHub Copilot (Claude Opus 4.6)
Generated-by: GitHub Copilot (Claude Opus 4.6) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
