fix: send and store real sub-second emission durations - #1374
Open
davidberenstein1957 wants to merge 1 commit into
Open
fix: send and store real sub-second emission durations#1374davidberenstein1957 wants to merge 1 commit into
davidberenstein1957 wants to merge 1 commit into
Conversation
The `duration` field was typed `int` in the pydantic schemas while the DB column and ORM were always `Float`, and `ApiClient.add_emission` dropped any measurement shorter than one second. 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 #1374 +/- ##
==========================================
+ Coverage 91.39% 91.43% +0.03%
==========================================
Files 49 49
Lines 5056 5053 -3
==========================================
- Hits 4621 4620 -1
+ 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 stands alone and can be reviewed and merged independently; #1203 will be rebased to drop the duplicated hunks.What
durationbecomesfloatinstead ofintincodecarbon/core/schemas.pyandcarbonserver/carbonserver/api/schemas.py(EmissionBase).ApiClient.add_emissionno longer refuses emissions shorter than one second, and no longer truncates the duration withint(...).ExperimentReport,ProjectReportandOrganizationReportwidendurationfrominttofloat.Why
The DB column and the ORM were always
Column(Float)(carbonserver/carbonserver/api/infra/database/sql_models.py) — only the pydantic models claimedint. So nothing about storage changes here; the schemas are simply being made honest about what the database already holds. On the client side the< 1guard silently dropped any measurement shorter than a second, which is every short-lived task.The report widening is not cosmetic: those endpoints
SUM()a Float column into a field declaredint. Today every stored duration happens to be a whole number, so it validates by luck. The first sub-second duration in the database makes the sum non-integral and the response model raises a validation error — a latent 500 on the experiment/project/organization report endpoints. Widening the reports is therefore part of this fix, not a separate cleanup.Tests
carbonserver/tests/api/test_schema_compatibility.py::test_millisecond_duration_survives_client_to_server— a client payload withduration=0.0042validates against the server schema unchanged.tests/test_api_call.py::TestApi::test_add_emission_sends_millisecond_duration_unchanged— replacestest_add_emission_skips_short_duration; asserts the POST body carries0.0042.Both fail on
master(ValidationError, andemissions not sent because of a duration smaller than 1) and pass with the fix.uv run pytest tests/ -q --ignore=tests/test_viz_data.py→ 626 passed, 21 skipped. carbonserver unit tests → 108 passed.pre-commit run --all-filesclean.🤖 Generated with Claude Code