Skip to content

fix: Keep minutes segment in format_duration for sub-minute remainders#1994

Merged
Pijukatel merged 1 commit into
apify:masterfrom
anxkhn:loop/crawlee-python__003
Jun 30, 2026
Merged

fix: Keep minutes segment in format_duration for sub-minute remainders#1994
Pijukatel merged 1 commit into
apify:masterfrom
anxkhn:loop/crawlee-python__003

Conversation

@anxkhn

@anxkhn anxkhn commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Description

format_duration (src/crawlee/_utils/time.py) drops the minutes segment for durations >= 1h that have zero full minutes but leftover seconds, because the minutes segment was appended only if minutes > 0. So a 1h0m30s duration rendered as 1h 30.0s, which reads as 1h30m. The sub-hour branch is already consistent (2min 30.0s); only the hours branch was wrong.

This appends the minutes segment whenever any sub-hour remainder exists, so the case now renders 1h 0min 30.0s. Seconds are still shown only when > 0, so durations with no remainder are unchanged (2h, 2h 1min).

format_duration feeds the user-facing FinalStatistics.to_table rows (crawler_runtime, request_total_duration), so the visible impact is the displayed stats string. to_dict uses total_seconds(), not this string, so machine-readable output is untouched.

Issues

  • None. Self-identified correctness fix; no tracked issue.

Testing

  • Adds tests/unit/_utils/test_time.py (parametrized) covering the buggy hour edge (1h0m30s, 1h0m1s) plus neighbours (2h, 2h1m, 2h1m30s, sub-minute, sub-second, exact minute, 0s, None). uv run pytest tests/unit/_utils/test_time.py passes; reverting the one source line fails exactly the two hour-edge cases. uv run poe lint and uv run poe type-check are clean.

Checklist

  • CI passed
When a duration was >= 1h with zero full minutes but leftover seconds, the minutes segment was dropped, so e.g. 1h0m30s rendered as '1h 30.0s' (reads as 1h30m). Append minutes whenever any sub-hour remainder exists, matching the minutes branch. Adds tests covering the buggy case and neighbours.

@Pijukatel Pijukatel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@vdusek want to take a look as well?

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Pijukatel Pijukatel merged commit b322ace into apify:master Jun 30, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants