Skip to content

Fix mypy failure on datadog timer wrapped by shared observability Timer - #69202

Merged
potiuk merged 1 commit into
apache:mainfrom
astronomer:fix-mypy-shared-observability-timer
Jul 1, 2026
Merged

Fix mypy failure on datadog timer wrapped by shared observability Timer#69202
potiuk merged 1 commit into
apache:mainfrom
astronomer:fix-mypy-shared-observability-timer

Conversation

@Lee-W

@Lee-W Lee-W commented Jul 1, 2026

Copy link
Copy Markdown
Member

Why

  • After the CI environment upgrade, prek run mypy-shared-observability fails: datadog_logger.py passes dogstatsd's TimedContextManagerDecorator into Timer(...), but real_timer is typed Timer | None, so the types don't match.
  • That annotation was never accurate — real_timer is never the shared Timer itself, it's a third-party backend timer (pystatsd / dogstatsd). It slipped through before only because pystatsd ships no stubs (treated as Any); datadog has stubs, which surfaced the mismatch.

What

  • Add a minimal BackendTimerProtocol (just start() / stop()) and retype Timer.__init__'s real_timer from Timer | None to BackendTimerProtocol | None, reflecting the interface it actually accepts.
  • Type-annotation-only fix, no runtime behavior change; both the pystatsd and dogstatsd timers are structurally compatible.

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: [Claude] following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

The real_timer parameter accepts any backend timer exposing start/stop
(pystatsd, dogstatsd), not the shared Timer itself. Typing it against a
minimal protocol reflects that contract and stops mypy rejecting the
typed dogstatsd TimedContextManagerDecorator.
@Lee-W
Lee-W requested review from amoghrajesh, ashb and potiuk as code owners July 1, 2026 11:02
@Lee-W
Lee-W marked this pull request as draft July 1, 2026 11:04
@potiuk
potiuk marked this pull request as ready for review July 1, 2026 12:06
@potiuk

potiuk commented Jul 1, 2026

Copy link
Copy Markdown
Member

This looks good. let me rebase ci upgrade :)

@potiuk
potiuk merged commit 5946046 into apache:main Jul 1, 2026
77 checks passed
xBis7 pushed a commit to xBis7/airflow that referenced this pull request Jul 7, 2026
…er (apache#69202)

The real_timer parameter accepts any backend timer exposing start/stop
(pystatsd, dogstatsd), not the shared Timer itself. Typing it against a
minimal protocol reflects that contract and stops mypy rejecting the
typed dogstatsd TimedContextManagerDecorator.

(cherry picked from commit 5946046)
potiuk added a commit that referenced this pull request Aug 1, 2026
…er (#69202) (#70887)

The real_timer parameter accepts any backend timer exposing start/stop
(pystatsd, dogstatsd), not the shared Timer itself. Typing it against a
minimal protocol reflects that contract and stops mypy rejecting the
typed dogstatsd TimedContextManagerDecorator.

(cherry picked from commit 5946046)

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
vatsrahul1001 pushed a commit that referenced this pull request Aug 5, 2026
…er (#69202) (#70887)

The real_timer parameter accepts any backend timer exposing start/stop
(pystatsd, dogstatsd), not the shared Timer itself. Typing it against a
minimal protocol reflects that contract and stops mypy rejecting the
typed dogstatsd TimedContextManagerDecorator.

(cherry picked from commit 5946046)

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants