Repository navigation
fix: avoid ZeroDivisionError when a CloudFetch download takes no measurable time - #977
Open
maharanay22 wants to merge 1 commit into
Open
maharanay22 wants to merge 1 commit into
maharanay22 wants to merge 1 commit into
Conversation
…urable time ResultSetDownloadHandler.run() timed each download with time.time() and _log_download_metrics() divided the byte count by that duration. On Windows with Python 3.12 and earlier, time.time() only advances about every 15.6 ms, so a chunk that downloads within one tick has a duration of exactly 0 and the whole fetch fails with ZeroDivisionError from a logging call. Measure the download with time.perf_counter(), which is monotonic and high resolution, and report the speed as inf when the duration is not positive so logging can never fail a download. Signed-off-by: Maha Rana Yadavalli <271375718+maharanay22@users.noreply.github.com>
This branch has not been deployed
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
1 participant
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.
What type of PR is this?
Description
ResultSetDownloadHandler.run()timed each CloudFetch download withtime.time(), and_log_download_metrics()divided the byte count by that duration. On Windows with Python 3.12 and earlier,time.time()only advances about every 15.6 ms, so a chunk that downloads within one tick has a measured duration of exactly 0. The whole fetch then fails withZeroDivisionError: float division by zerofrom a logging call. With an instant mock HTTP client, 196 of 200 downloads failed this way on Windows 11 / Python 3.12.This PR measures the download with
time.perf_counter(), which is monotonic and high resolution, and logs the speed asinfwhen the duration is not positive, so logging can never fail a download. The log format is otherwise unchanged, and no slow-download warning is emitted in that case. I usedperf_counterrather thantime.monotonicon purpose, becausemonotonichas the same ~15.6 ms resolution on Windows with Python 3.12 and earlier.How is this tested?
New
test_run_successful_when_clock_does_not_advancefreezes the clock and runsrun()without patching_log_download_metrics. It fails onmainwithZeroDivisionError: float division by zeroand passes with this change. Full unit suite: 1021 passed, 5 skipped. Manually, the 200-download loop above now succeeds 200 of 200 times on Windows 11 / Python 3.12.Related Tickets & Documents
The speed logging was added in #654. The existing tests in
tests/unit/test_downloader.pypatch out_log_download_metrics"to avoid division by zero"; I left those in place to keep this PR small.