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
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>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1375 +/- ##
==========================================
+ Coverage 91.39% 91.44% +0.04%
==========================================
Files 49 49
Lines 5056 5061 +5
==========================================
+ Hits 4621 4628 +7
+ Misses 435 433 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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