Repository navigation
fix(kernels): stream kernels output files through download_file - #1206
Merged
Merged
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
kernels_output fetched each output file with requests.get(stream=True) and then wrote response.content, which reads the whole body into memory. It had no read timeout, no retry and no size check, and it opened the target file before reading the body, so a dropped connection failed the command and left an empty file under the final name. Dataset and competition downloads already go through download_file: chunked writes, a Content-Length check, retries that resume with a Range request, and a 300 s read timeout. kernels_output now uses it too. Measured with an in-process HTTP server: a 512 MiB output raised the client's peak RSS by about 1,175 MB before and 7 MB after, and a connection cut at 1.5 MiB of a 3 MiB file now resumes from the first 1 MiB chunk instead of failing.
nefayran
force-pushed
the
fix/kernels-output-stream
branch
from
October 1, 2026 00:41
47e5661 to
67f6856
Compare
Contributor
|
/gcbrun |
stevemessick
self-requested a review
October 1, 2026 21:31
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.
Summary
kaggle kernels outputnow saves each file throughdownload_file, the path dataset and competition downloads already take. Files are written in chunks instead of held in memory, checked against Content-Length, and resumed after a dropped connection.Problem
kernels_outputfetches each file withrequests.get(url, stream=True)and writesresponse.content, which reads the whole body into memory. The request has no read timeout, nothing retries it, and nothing compares the written size with Content-Length. The target file is opened before the body is read, so when the connection drops, the command fails and leaves an empty file under the final name.Measured with an in-process HTTP server (macOS arm64, Python 3.14, requests 2.34.2):
ChunkedEncodingError, 0-byte file left on diskSolution
Call
download_filefor kernel output files and passtimeout=300to the first request, the same read timeoutdownload_fileuses for its retries. Resume is on unless--forceis given, as indatasets download.download_filedoes not use itshttp_clientargument, and the client opened for the output listing is closed by the time the files are fetched, so the call passesNone.Tests
tests/unit/test_kernels_output_download.pyrunskernels_outputagainst a local HTTP server that supports Range requests. One test checks that the file goes throughdownload_fileand arrives intact. The other cuts the first connection halfway through the second 1 MiB chunk and checks that the retry asks forbytes=1048576-and that no.kaggle-partialmarker is left. Both fail on main.test_kernels_output_paths.pyand two pagination tests intest_kernels_logs.pyfaked the old.contentread. They now stubdownload_file, and therequests.getassertion includes the timeout.pytest tests/unit: 1397 passed.black --checkandmypyreport no issues.