Uh oh!
There was an error while loading. Please reload this page.
fix: reject powermetrics on non-Apple-Silicon Macs - #1333
fix: reject powermetrics on non-Apple-Silicon Macs#1333davidberenstein1957 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 #1333 +/- ##
==========================================
+ Coverage 91.43% 91.45% +0.02%
==========================================
Files 49 49 Lines 5057 5058 +1 ==========================================
+ Hits 4624 4626 +2 + Misses 433 432 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
4cc0e0d to
d1121f5Compare_setup_cli returned without setting _cli and without raising on an Intel Mac, so ApplePowermetrics() succeeded there and ResourceTracker could register an AppleSiliconChip on hardware that has none. It also crashed with AttributeError when detect_cpu_model() returned None. Raise SystemError on both paths, use the validated self._cli in the argv, drop a stray empty argument, and give subprocess.call a timeout so a hung powermetrics cannot block the measurement thread. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d1121f5 to
132f230Comparedavidberenstein1957
commented
Aug 19, 2026
Closing as folded into #1345. Both are about ApplePowermetrics being dishonest in two ways: it constructs successfully on hardware it cannot measure, and it returns NaN when it measures nothing. Same file, same class, adjacent methods, and they collided textually while both extending tests/test_powermetrics.py, so they are now one 'harden powermetrics' review. The subprocess timeout that was bundled in here has been split out separately as #1397, since it is an unrelated change with its own failure mode. |
What changed
ApplePowermetrics._setup_cli()now raises instead of falling through:SystemError(previously the method returned without assigningself._cliand without raising, soApplePowermetrics()succeeded on Intel Macs);detect_cpu_model()returningNoneraises the sameSystemErrorrather than an unexpectedAttributeError;self._cliis assigned on the one path that validated the executable._log_values()now usesself._cliinstead of a hardcoded"powermetrics", so the validated path is the one actually executed. Two smaller defects in the same argv are fixed alongside it: a stray""positional argument passed verbatim topowermetrics, and the missing timeout onsubprocess.call, which let a hungpowermetricsblock the measurement thread indefinitely.Why
Because
_setup_cli()never raised on Intel Macs,is_powermetrics_available()returnedTruewhenever a permissive sudo rule existed, and_try_platform_cpu_backend(codecarbon/core/resource_tracker.py:225) routed those machines into_setup_powermetrics, registering anAppleSiliconChipon hardware that has none. Withdetect_cpu_model()returningNone, the resultingAttributeErrorwas swallowed by the broadexcept Exceptioninis_powermetrics_available()and reported as powermetrics simply being unavailable, hiding genuine cpuinfo failures.Note for the changelog: machines that were accidentally using powermetrics on Intel Macs now fall through to Intel Power Gadget or the CPU-load fallback, which is the intended routing but does change their tracking method.
How it was verified
Four new tests in
tests/test_powermetrics.pycover the Intel Mac rejection, theNoneCPU model, the clean argv with a timeout, and the timeout warning path. All four fail onmasterand pass with this change; the fulltests/test_powermetrics.pyandtests/test_resource_tracker.pysuites pass (51 tests).ruff format --checkis clean on both touched files.Closes#1313
🤖 Generated with Claude Code