Update the default UPLOAD_CHUNK_SIZE to 100MB. #47
Conversation
The goal here is to lower latency for transfers, as well as having some consistency between client libraries. For more discussion, see googleapis/google-api-python-client#482.
|
RE: download chunk size, these are the two most relevant spots:
It may be set somewhere in |
|
This LGTM, but @jonparrott should LGTM too before a merge. |
|
Eek, lots of "256 KB must divide chunk size" errors. |
The validation of `ResumableUpload.chunk_size` checks divisibility by a constant, and then provides an error with a hardcoded value for that constant. This can be confusing (eg googleapis#47). The fix here is to just insert the appropriate value into the error message. Adding a bad value still gives the expected error message, eg ``` Traceback (most recent call last): File "/usr/local/google/home/craigcitro/gh/google-resumable-media-python/tests/unit/test__upload.py", line 245, in test_constructor upload = _upload.ResumableUpload(RESUMABLE_URL, 12 * 1024)#chunk_size) File "/usr/local/google/home/craigcitro/gh/google-resumable-media-python/google/resumable_media/_upload.py", line 323, in __init__ resumable_media.UPLOAD_CHUNK_SIZE / 1024)) ValueError: 256 KB must divide chunk size ```
|
So looking at the way that constant is used, I think I had misunderstood its purpose. AFAICT, it's not used as the default for chunk sizes when starting an upload. Instead, the caller always provides Is that correct? If so, I'll drop this PR, but my offer to tweak default chunksizes can be happily reapplied to (The error message above was misleading, due to the string |
|
@craigcitro No that isn't correct. The actual storage API requires chunks are divisible by some value (I can't recall what it is). |
|
@dhermes We're maybe missing each other on this one. :) Storage does indeed have a requirement on chunk size being divisible by 256KB: from the docs, "Create chunks in multiples of 256 KB (256 x 1024 bytes) in size, except for the final chunk that completes the upload." (This same requirement holds for media transfers across all APIs.) Now, in the My original goal in this CL was to change the default chunk size, but I think that was misguided: there is no default chunk size. If such a default exists, it's presumably in the callers of this library. More directly:
|
|
@craigcitro, this is an old PR. Can we close it? |
|
Sure. |
4 participants
The goal here is to lower latency for transfers, as well as having some
consistency between client libraries.
For more discussion, see
googleapis/google-api-python-client#482.
PTAL @dhermes -- I also didn't see an obvious place you were setting a download chunk size, but if you point me at one I'm happy to update it. 😁