[davidberenstein1957]

Description

ApiClient._request called bare requests.get/post/patch with a hardcoded timeout=2 and no Session, so every emission POST opened a new TCP connection. This PR:

• Uses one requests.Session per ApiClient, so the socket and TLS handshake are reused. ApiClient.close() closes it, and CodeCarbonAPIOutput.exit() calls it on tracker stop.
• Replaces timeout=2 with a (3.05, 10) connect/read timeout.
• Stamps each emission with the time it was measured (EmissionsData.timestamp) instead of the time it was sent. A missing or unparseable timestamp falls back to now.
• Adds a 60 s cooldown after a failed run creation. _create_run re-raises every failure, so _emit stops after the first attempt, and with the cooldown a down API costs one call per minute instead of one per measurement tick. out(), which runs on flush()/stop(), bypasses this cooldown and tries run creation once, so the last emission is not dropped; periodic live_out() ticks respect it and log the skip at DEBUG.
• Known limit, found in an end-to-end run against a real API and Postgres: the final flush now creates the run, but if the last window is shorter than 1 s the existing duration < 1 check still drops that row. fix: send and store real sub-second emission durations #1374 removes that check, so merge fix: send and store real sub-second emission durations #1374 before or with this PR.

An earlier version of this description mentioned a Retry adapter and api_timeout / api_retries config options; neither is in the current diff. The measurement-timestamp change from #1341 (now closed) is included here.

Related Issue

Part of #1338

Motivation and Context

Without connection reuse, every emission POST paid a TCP (and TLS) handshake. When the API is down, each tick used to retry run creation and wait for the timeout. Emissions sent late were stamped with the send time, which shifted them on the dashboard.

How Has This Been Tested?

• tests/test_api_client_session.py (new): runs against a stdlib loopback server rather than requests_mock, which bypasses the transport layer. It covers sequential calls reusing one connection, close() being idempotent, and requests getting the connect/read timeout.
• tests/test_api_call.py: test_add_emission_keeps_measurement_timestamp (naive, missing and unparseable timestamps) and test_failed_create_run_is_not_retried_during_cooldown (no retry inside the cooldown, retry after it).

CI is green on this branch.

Screenshots (if appropriate):

N/A

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

AI Usage Disclosure

• 🟥 AI-vibecoded
• 🟠 AI-generated
• ⭐ AI-assisted
• ♻️ No AI used

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the docs/how-to/contributing.md document.
• I have added tests to cover my changes.
• All new and existing tests passed.

Deferred: making the tracker's own timestamp timezone-aware (to fix the DST edge case). That changes the CSV timestamp format, so it will ship in the next minor release together with #1334. Not included: moving the direct requests.get calls in electricitymaps_api / geography.py onto the shared session.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.74%. Comparing base (e159a68) to head (0c7231b).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1340      +/-   ##
==========================================
+ Coverage   93.66%   93.74%   +0.07%     
==========================================
  Files          50       50              
  Lines        5591     5613      +22     
==========================================
+ Hits         5237     5262      +25     
+ Misses        354      351       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[benoit-cty]

🤖 This review comment was written and posted by Claude Opus 5.5 (AI assistant), at the request of @benoit-cty. Findings were checked by reading the code and running tests locally (merged with current master where relevant), but please double-check before acting on them.

Verdict: ✅ Approve with nits (please merge #1339 first)

Session reuse (with close() wired into CodeCarbonAPIOutput.exit()), the (3.05, 10) connect/read timeouts, and stamping emissions with EmissionsData.timestamp all look good. 80 tests pass, and the loopback test that counts connections is nice. There are no retries at all, so there is no risk of a replayed POST creating duplicate emissions.

Issues:

1. The description doesn't match the code. It describes:

   • a Retry adapter with a split POST vs GET/PATCH policy
   • api_timeout / api_retries config, and docs for them
   • a run-creation backoff guard
   • tests/test_api_client_retry.py
   • "stacked on fix(api): stamp emissions with measurement time, not send time #1341" (now closed)

   None of these are in the diff. Please update the description, or add the missing parts.

2. A hung API now blocks the measurement thread much longer (api_client.py:37, output_methods/http.py:62-68).

   • When run_id is None, _emit calls _ensure_api_run() and then add_emission(), and each can call _create_run.
   • Against a hung API, one live_out can block the scheduler thread for about 2 × 13 s ≈ 26 s, versus about 4 s on master. That is longer than the default 15 s tick.
   • Suggested fix: at most one _create_run attempt per _emit, plus a cooldown after a failed run creation (the "backoff guard" from the description).
   • Together with refactor(scheduler): run ticks on one daemon thread instead of chained Timers #1339 (5 s join cap in stop()), this also makes it more likely that the final flush overlaps an in-flight tick.

Nit:

• _measurement_timestamp (api_client.py:40) takes a naive, second-precision local time and calls .astimezone() on it. In the repeated hour at the DST fall-back, the offset can be wrong by an hour. Creating a timezone-aware timestamp at the source (emissions_tracker.py:1076, e.g. datetime.now().astimezone()) avoids that.

[davidberenstein1957]

Made the changes in 72c53b7: 60 s cooldown after a failed run creation.

On the 2 x 13 s concern: _create_run re-raises every failure, so _emit stops after the first attempt; with the cooldown a down API now costs one ~13 s call per minute, not per tick. Stays compatible with #1339 (no shared files).

Not done: rewriting the description (my edit was blocked; the retry/config/backoff sections are stale, the diff is Session + timeouts + measurement timestamp + cooldown). Skipped the DST nit: making the tracker timestamp offset-aware changes the CSV timestamp format, which needs a maintainer call.

[davidberenstein1957]

Deferring the DST timestamp nit: making the timestamp timezone-aware at the source changes the CSV timestamp format, so it will ship in the next minor release together with #1334 rather than in this PR.

[benoit-cty]

Rebased onto master (which now includes #1339) and reviewed. New head: 0c7231b.

Rebase

• The rebase had no conflicts. The 4 original commits are kept as c50bbb3, 7b11bbe, c032db1 and 9e124b4.

Fix added: 0c7231b, refactor(api)

The extra send of the last emission on stop() was more complicated than it needed to be:

• stop() made a second pass over the output handlers with out(final=True).
• It used monotonic timestamps to avoid trying to create the run twice.
• add_emission had its own final flag and cooldown check.

out() only runs on an explicit flush() or stop(), while the periodic ticks go through live_out(). So I made these changes:

• CodeCarbonAPIOutput.out() now always bypasses the cooldown.
• While run creation is in its cooldown, _emit skips the emission and logs it at DEBUG.
• emissions_tracker.py is now identical to master.
• add_emission no longer has the final flag. If it creates the run itself, it still sends the emission.
• With the API down, stop() still tries to create the run only once.
• Behaviour change: flush() now also bypasses the cooldown, not just stop(). Each explicit flush() while the API is down can block for up to one connect/read timeout.
• Tests updated, plus new ones for: the skip during the cooldown, exit() closing the session, and add_emission sending after it creates the run.

Review feedback

• The code now matches the description. I changed one sentence in the body, the one about bypassing the cooldown.
• The hung-API blocking is handled by the cooldown.

Tests and coverage

• 87 tests pass in the API, HTTP, session and tracker test files.
• 100% of the changed lines in codecarbon/ are covered. http.py is at 100%, and the only uncovered lines in api_client.py were already uncovered on master.
• pre-commit passes.
• Full suite: 836 passed, 7 failed. Similar failures occur when the same files run on master in this sandbox: the RAPL permission tests fail because it runs as root, and the flush/logging tests fail depending on test order (the one that differed passes when run alone).

Docs: no change needed.

Left for the maintainer

• The DST nit (timezone-aware tracker timestamp) is deferred to the next minor release, with fix: raise on offline tracker configuration errors #1334.
• fix: send and store real sub-second emission durations #1374 still needs to be merged before or with this PR, because master still has the duration < 1 check.

---

Generated by Claude Code