regarmukesh3g opened a new pull request, #72576:
URL: https://github.com/apache/airflow/pull/72576
`Timer` in `shared/observability/.../metrics/protocols.py` declares two
attributes but never creates them:
```python
class Timer(TimerProtocol):
_start_time: float | None # bare annotation — creates no attribute
duration: float | None # bare annotation — creates no attribute
def __init__(self, real_timer=None):
self.real_timer = real_timer # the only attribute actually set
```
A bare annotation at class level records a type in `__annotations__` for type
checkers; it does not create an attribute. So after `Timer()` the instance
has
only `real_timer` — `_start_time` and `duration` do not exist until `start()`
and `stop()` assign them.
### Why this is a bug rather than a "call start() first" contract
`stop()` already guards its duration calculation:
```python
def stop(self, send: bool = True) -> None:
if self._start_time is not None: # reads a possibly-nonexistent
attribute
self.duration = 1000.0 * (time.perf_counter() - self._start_time)
```
That `is not None` check only makes sense if `_start_time` is expected to
sometimes *be* `None` — i.e. "the timer was never started, skip the math".
The
intent is clearly to tolerate that state. But `_start_time` can never be
`None`:
it is either missing entirely (before `start()`) or a float (after). The
guard
can therefore never do what it was written to do, and raises instead.
The sibling `_DualTimer` in the same package already initializes both
attributes
in its constructor (`self.duration: float | None = None`), so this also
brings
`Timer` in line with it.
### Consequences
- `stop()` on a timer that was never started raises
`AttributeError: 'Timer' object has no attribute '_start_time'`.
- Reading `t.duration` on a timer that never ran raises `AttributeError`,
even
though the class docstring documents reading `duration` as normal usage.
This matters on error paths that stop a timer they may not have started. The
scheduler stops its timers inside `except OperationalError` blocks
(`scheduler_job_runner.py:851` and `:2081`):
```python
try:
...
timer.stop(send=True)
except OperationalError as e:
timer.stop(send=False) # AttributeError here masks the real DB error
raise e
```
If the failure happens before the timer starts, the `AttributeError` raised
from
inside the handler replaces the `OperationalError` actually being handled.
### The change
Initialize both attributes in `__init__`, so the existing guard can work:
```python
self._start_time = None
self.duration = None
```
Normal start/stop usage is unaffected.
### Testing
Adds `TestTimer` covering the un-started states. All four tests fail with
`AttributeError` without the change and pass with it. The rest of
`test_stats.py` (101 tests) is unaffected.
Note:
`test_otel_logger.py::TestOtelMetrics::test_reinit_after_fork_exports_metrics`
fails locally, but it fails identically on unmodified `main` and is
unrelated to
this change.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
Generated-by: Claude Code (Opus 5) 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]
