Use time.monotonic() for duration checks in databricks provider#69758
Use time.monotonic() for duration checks in databricks provider#69758magic-peach wants to merge 1 commit into
Conversation
Duration and timeout comparisons should use time.monotonic() instead of time.time() to be immune to system clock adjustments (NTP, DST, etc.). This aligns with the project coding standard documented in AGENTS.md. Also fixes a grammar error in a comment: 'important steps; if a query takes to log' -> 'important step; if a query takes too long'. Signed-off-by: Akanksha Trehun <akankshatrehun@gmail.com>
|
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
|
SameerMesiah97
left a comment
There was a problem hiding this comment.
Besides failing CI, I think this may introduce the same issue that was previously fixed in the dbt Cloud provider (#67616).
In the deferrable path, end_time is computed in the worker and passed to DatabricksSQLStatementExecutionTrigger, where it is later compared by the trigger. If end_time is changed to time.monotonic(), the value is no longer valid across process boundaries because monotonic clocks are local to each process.
We previously had to revert a similar change in the dbt Cloud provider for this reason. Could we keep time.time() for the serialized end_time, and only use time.monotonic() for duration measurements that remain entirely within a single process? In other words, would it be better to just keep this change limited to just the hook and leave time.time() as is in the mixin and the trigger?
Duration and timeout comparisons should use
time.monotonic()instead oftime.time()to be immune to system clock adjustments (NTP, DST, etc.). This aligns with the project coding standard documented in AGENTS.md.Changes:
databricks_base.py: Replacetime.time()withtime.monotonic()in Azure metadata cache TTL checks (both sync and async versions)databricks.pytrigger: Replacetime.time()withtime.monotonic()in the polling loop timeout checkmixins.py: Replacetime.time()withtime.monotonic()in SQL statement execution timeout checks, and fix grammar error in comment ("important steps; if a query takes to log" → "important step; if a query takes too long")Note:
time.time()is correctly retained for token expiry comparisons (lines 337, 378, 640, 1021, 1066) because those compare against wall-clock Unix timestamps received from the Databricks server.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.7) following the guidelines
Drafted-by: Claude Code (Opus 4.7) (no human review before posting)