fix: resume the scheduler in stop_task only if start_task paused it - #1375
Open
davidberenstein1957 wants to merge 1 commit into
Open
fix: resume the scheduler in stop_task only if start_task paused it#1375davidberenstein1957 wants to merge 1 commit into
davidberenstein1957 wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1375 +/- ##
=======================================
Coverage 91.43% 91.44%
=======================================
Files 49 49
Lines 5057 5062 +5
=======================================
+ Hits 4624 4629 +5
Misses 433 433 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
davidberenstein1957
marked this pull request as ready for review
August 13, 2026 05:05
davidberenstein1957
force-pushed
the
fix/stop-task-scheduler-resume
branch
from
August 19, 2026 09:18
3609604 to
70b7d66
Compare
start_task stops the periodic scheduler but nothing ever restarted it, so a tracker started with start() lost its periodic measurements after the first task. Restarting it unconditionally would instead leave a 1s scheduler running for pure start_task/stop_task users, so track whether start_task actually paused a running scheduler. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
davidberenstein1957
force-pushed
the
fix/stop-task-scheduler-resume
branch
from
August 19, 2026 14:29
70b7d66 to
f57ceb6
Compare
davidberenstein1957
added a commit
that referenced
this pull request
Aug 19, 2026
Adds an ASGI middleware that gives each HTTP request its share of a long-running tracker's energy, plus the attribution model behind it. One tracker runs for the app's lifetime. Each completed sampling window (t_prev, t_now, dE) is split across the requests in flight during it, weighted by their overlap with the window and normalised by the sum of the weights. Windows with nothing in flight are recorded as unattributed. The invariant attributed + unattributed == settled holds exactly after every window, and is what the concurrency test pins down. Why not per-request start/stop energy snapshots: with N requests in flight each request observes the whole machine's delta, so the sum overcounts by roughly N - measured up to 88x at 100 concurrent requests. Fair-share weighting is the only split that conserves the run total. A request's share is only known one or more sampling windows after its response was sent, so results are reported then, via a callback. A request that never covered a completed window reports energy_kwh=None rather than zero: there is no honest number for it. Tracker side: add_energy_window_observer / remove_energy_window_observer expose the sampling windows, and http_request_emissions() scales the run's EmissionsData down to one attributed share using the run's accumulated component ratios and carbon intensity. Depends on #1374 (duration int -> float in the emissions schemas, and dropping the duration < 1 send guard) and #1375 (scheduler pause handling around tasks). Both are carried by their own PRs rather than duplicated here, so this should merge after them. Deliberately left out, to keep the diff reviewable: hardware-tier gating of which backends can resolve a sampling window, include/exclude path filtering (endpoint labelling is two lines inline), idle-baseline subtraction, per-endpoint aggregation, routing per-request rows into the tracker's own CSV/API output handlers, a lifespan helper, and a dedicated docs page. Each is additive on top of this and can follow if there is demand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
davidberenstein1957
added a commit
that referenced
this pull request
Aug 19, 2026
Adds an ASGI middleware that gives each HTTP request its share of a long-running tracker's energy, plus the attribution model behind it. One tracker runs for the app's lifetime. Each completed sampling window (t_prev, t_now, dE) is split across the requests in flight during it, weighted by their overlap with the window and normalised by the sum of the weights. Windows with nothing in flight are recorded as unattributed. The invariant attributed + unattributed == settled holds exactly after every window, and is what the concurrency test pins down. Why not per-request start/stop energy snapshots: with N requests in flight each request observes the whole machine's delta, so the sum overcounts by roughly N - measured up to 88x at 100 concurrent requests. Fair-share weighting is the only split that conserves the run total. A request's share is only known one or more sampling windows after its response was sent, so results are reported then, via a callback. A request that never covered a completed window reports energy_kwh=None rather than zero: there is no honest number for it. Tracker side: add_energy_window_observer / remove_energy_window_observer expose the sampling windows, and http_request_emissions() scales the run's EmissionsData down to one attributed share using the run's accumulated component ratios and carbon intensity. Depends on #1374 (duration int -> float in the emissions schemas, and dropping the duration < 1 send guard) and #1375 (scheduler pause handling around tasks). Both are carried by their own PRs rather than duplicated here, so this should merge after them. Deliberately left out, to keep the diff reviewable: hardware-tier gating of which backends can resolve a sampling window, include/exclude path filtering (endpoint labelling is two lines inline), idle-baseline subtraction, per-endpoint aggregation, routing per-request rows into the tracker's own CSV/API output handlers, a lifespan helper, and a dedicated docs page. Each is additive on top of this and can follow if there is demand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extracted from #1203 (
feat/add-fastapi-middleware), which bundled this with unrelated FastAPI middleware work. It changes behaviour for everystart_taskuser, so it deserves review on its own rather than buried in a 3,700-line feature branch. #1203 will be rebased to drop the duplicated hunks.What
start_taskstops the periodic measurement scheduler so it does not interfere with the task measurement, but nothing ever restarted it. A tracker started withstart()therefore lost its periodic measurements permanently after the first task.Restarting it unconditionally in
stop_taskwould be wrong in the other direction: users who only ever callstart_task/stop_task(neverstart()) would be left with a 1s scheduler running that nobody asked for and nothing stops.So
start_taskrecords whether it actually paused a running scheduler (_scheduler_paused_by_task = not self._scheduler._stopped), andstop_taskresumes only in that case.Behaviour change
start()+start_task()/stop_task(): periodic measurement now continues after the task, as it did before the task started. Previously it stayed dead.start_task()/stop_task()only: unchanged — no scheduler left running.Tests
tests/test_emissions_tracker.py::TestCarbonTracker::test_stop_task_resumes_scheduler_only_if_start_task_paused_itcovers both directions. It fails onmasterand passes with the fix.uv run pytest tests/ -q --ignore=tests/test_viz_data.py→ 627 passed, 21 skipped.pre-commit run --all-filesclean.🤖 Generated with Claude Code