Repository navigation
fix: raise on offline tracker configuration errors - #1334
davidberenstein1957 wants to merge 9 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1334 +/- ##
==========================================
+ Coverage 93.66% 93.85% +0.18%
==========================================
Files 50 50
Lines 5591 5610 +19
==========================================
+ Hits 5237 5265 +28
+ Misses 354 345 -9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
85eedc8 to
3d7c6ec
Compare
df16736 to
7b8487b
Compare
Verdict: 💬 Needs a maintainer decision (leaning approve, for a minor release)The bug is real. A bad Behaviour change to be aware of:
Nit:
|
|
Made the changes in e77657f: Not done, pending a maintainer decision: whether |
|
Made the changes in 1e238ba: |
`@suppress(Exception)` on `OfflineEmissionsTracker.__init__` swallowed configuration errors such as a missing `output_dir`, returning a half-built object with no `_start_time`, `_hardware` or `_scheduler`. `start()` and `stop()` then failed silently and no emissions were recorded at all. Construction now raises, matching `EmissionsTracker`. The suppression on `start`/`flush`/`stop` is kept, so runtime measurement errors still cannot crash a user's job. The behaviour change is noted in the docs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Asserts are stripped under python -O, so a non-string region or country_2letter_iso_code would pass silently. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
track_emissions now logs construction errors and calls the function without tracking, matching start_task. Direct construction still raises. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…enting __init__ Wrapping the whole BaseEmissionsTracker.__init__ body in try/except re-indented ~60 lines, which already conflicted with master on rebase and will conflict with every future change to __init__. A small decorator gives the same behaviour with a two-line diff to __init__. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JfRKgUd281URY6fMyNAQmh
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JfRKgUd281URY6fMyNAQmh
7e7de76 to
9f42c68
Compare
|
Rebased on
For the maintainer:
Generated by Claude Code |
Description
Removes
@suppress(Exception)fromOfflineEmissionsTracker.__init__incodecarbon/emissions_tracker.py, so configuration errors raised during construction propagate to the caller instead of being swallowed. Also wraps the body ofBaseEmissionsTracker.__init__(after_configure_multiple_runs) intry/except Exception, releasingself._lockif it was acquired before re-raising, so a constructor failure withallow_multiple_runs=Falsedoes not leave a stale lock blocking the next tracker. The offline-onlyregionandcountry_2letter_iso_codetype checks, which run beforesuper().__init__, now raiseTypeErrorwith a descriptive message instead of a bareassert. In@track_emissions, tracker construction is now wrapped in its owntry/except: on failure, the error is logged and the decorated function still runs untracked (the call tofn(*args, **kwargs)sits outside theexceptblock, so the construction error is not chained via__context__/exc_infoto any exception the function itself raises).Related Issue
Fixes #1311
Motivation and Context
The decorator swallowed configuration errors raised during construction, most visibly the
OSErrorfrom_set_from_confwhenoutput_dirdoes not exist. The constructor then returned an object that had never reached_initialize_runtime_state()/_initialize_scheduler_state(), so it had no_start_time,_hardware,_scheduler, or_output_handlers. The subsequentstart()andstop()calls are themselves suppressed, so they degraded intoAttributeErrorwarnings, and the run finished with no emissions file and no exception. Under the CLI defaultlog_level="error", nothing was printed at all. The onlineEmissionsTrackeralready raises for the same input, so this also removes an asymmetry between the two constructors. Suppression onstart,flush, andstopis deliberately left in place, since runtime measurement errors must never crash a user job; only construction, where the object invariants were never established, now fails loudly. This is a behavior change at a public boundary and is intended to ride a minor release (3.4.0), not a patch.How Has This Been Tested?
Added
test_offline_tracker_raises_on_invalid_output_dirintests/test_offline_emissions_tracker.py, asserting both the offline and the online tracker raiseOSErrorfor a non-existentoutput_dir. It fails on master and passes with this change.test_offline_tracker_raises_on_invalid_regioncovers the offline-only region check (TypeErrorfor a non-stringregion), andtest_offline_tracker_raises_on_invalid_country_2letter_iso_codecovers the same forcountry_2letter_iso_code; both new tests fail if@suppress(Exception)is restored on the constructor.test_offline_tracker_lowercases_valid_regionandtest_offline_tracker_uppercases_valid_country_2letter_iso_codeguard against the stricter checks rejecting valid input.test_constructor_failure_releases_lock(intests/test_emissions_tracker.py) asserts a constructor failure withallow_multiple_runs=Falsereleases the lock file so a subsequent tracker can start.test_track_emissions_runs_function_when_tracker_construction_failsandtest_untracked_function_errors_are_not_chained_to_construction_errorcover the@track_emissionsdecorator: the function still runs untracked on a construction failure, and the construction error is not chained to an exception the function itself raises.Screenshots (if appropriate):
N/A
Types of changes
AI Usage Disclosure
Checklist:
CHANGELOG.mddoes not exist on master yet, so the release note lives indocs/reference/api.mdunder "Constructor errors (changed in v3.4.0)":OfflineEmissionsTrackerno longer swallows exceptions raised while constructing the tracker; configuration errors (e.g. a non-existentoutput_dir) now propagate to the caller, matchingEmissionsTracker. Suppression onstart/flush/stopis unchanged.