Fix convert_to_dapr_duration double-counting sub-second time - #1192
Fix convert_to_dapr_duration double-counting sub-second time#1192amdadulbari wants to merge 2 commits into
Conversation
convert_to_dapr_duration() derived the seconds field from
divmod(td.total_seconds(), 60), so it still carried the sub-second
fraction that is also emitted via the ms/us fields, and formatting it
with {:.0f} rounded a fraction >= 0.5 up into an extra whole second. As a
result timedelta(milliseconds=1500) serialized to '0h0m2s500ms0us' (2.5s)
and did not round-trip.
Truncate the whole seconds with int() so the sub-second fraction is only
counted once (via ms/us). Add a regression test.
Signed-off-by: Md. Amdadul Bari Imad <amdadulbari@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes convert_to_dapr_duration() in the Dapr Python SDK serializer utilities so that timedelta values with a sub-second fraction >= 0.5s no longer round up and double-count the fractional part when emitting the Go/Dapr duration string format.
Changes:
- Truncate the whole-seconds component when formatting durations so sub-second fractions are emitted only via the
ms/μsfields. - Add a regression test asserting both the exact encoded string and an exact round-trip for a
timedelta(milliseconds=1500)case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
dapr/serializers/util.py |
Fixes duration formatting by truncating the seconds field to avoid double-counting/rounding sub-second fractions. |
tests/serializers/test_util.py |
Adds a regression test covering sub-second rounding/double-counting and validates exact round-trip behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1192 +/- ##
=======================================
Coverage 82.77% 82.77%
=======================================
Files 123 123
Lines 10130 10130
=======================================
Hits 8385 8385
Misses 1745 1745 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| duration = convert_to_dapr_duration(timedelta(milliseconds=1500)) | ||
| self.assertEqual(duration, '0h0m1s500ms0μs') | ||
| self.assertEqual(convert_from_dapr_duration(duration), timedelta(milliseconds=1500)) | ||
|
|
There was a problem hiding this comment.
Nit: I think it'd be nice to cover 1999 and 1001 milliseconds too.
Description
convert_to_dapr_duration()produces a Dapr duration string that is ~1 second too large for anytimedeltawhose sub-second part is>= 0.5.The
secondsfield comes fromdivmod(td.total_seconds(), 60), so it still carries the sub-second fraction — which is also emitted via thems/μsfields (fromtd.microseconds). Formatting it with{:.0f}then rounds a fraction>= 0.5up into an extra whole second, so the fraction is both double-counted and rounded.This is on the serialization hot path (
DaprJSONEncoderserializes everytimedelta), so any duration with a>= 0.5sfraction — e.g. actor timer/reminder periods or state TTLs — is encoded incorrectly.Fix
Truncate the whole-seconds field with
int()so the sub-second fraction is only counted once (via thems/μsfields).hours/minsare always whole, so onlysecondsneeds this.Tests
Added
test_convert_timedelta_to_dapr_duration_subsecondintests/serializers/test_util.py(asserts the exact string and an exact round-trip). It fails before this change and passes after. Existing tests,ruff, andmypyall pass.