tree: Implement upload_fobj protocol to all trees #5307
Conversation
isidentical
changed the title
dvc/tree/gdrive.py
Outdated
There was a problem hiding this comment.
This is a bit unclear, internally pydrive2 uses MediaIoBaseUpload
https://github.com/iterative/PyDrive2/blob/ee368315132bd31bb643bf3d8af088cc512e39f3/pydrive2/files.py#L776-L778
the initializer of this wrapper says chunk size should be lower than 5MB
https://googleapis.github.io/google-api-python-client/docs/epy/googleapiclient.http.MediaIoBaseUpload-class.html#__init__
though the default chunk size is 100 MB (if I am not missing anything) (line #4721 https://googleapis.github.io/google-api-python-client/docs/epy/googleapiclient.http-pysrc.html#MediaIoBaseUpload.__init__
cc: @shcheklein
There was a problem hiding this comment.
Yes, information is confusing, probably they forgot to update the comment, or it should be interpreted in some other way. I see for example, that rclone has 8MB - https://forum.rclone.org/t/google-drive-and-optimal-drive-chunk-size/1186/11 ... and people mention 256M when they deal with very large files. I think it would be a good option to have, and we might indeed consider making it smaller by default to reduce memory pressure.
There was a problem hiding this comment.
we use 64 MiB on all other providers, though I think there is no way (as of now) to pass the chunk_size into MediaIoBaseUpload. Maybe we should consider adding it as a feature to pydrive2?
isidentical
force-pushed
the
azure-multipart
branch
from
e357e85 to
1875155
Compare
|
Whoa, something strange is going on with the tests. I don't think it is related to your PR. |
|
@isidentical Fixed the issue in #5333 . Please rebase. |
isidentical
force-pushed
the
azure-multipart
branch
2 times, most recently
from
799b013 to
ce5579a
Compare
isidentical
force-pushed
the
azure-multipart
branch
from
ce5579a to
1a43bfc
Compare
|
|
||
| # Needed for some providers, and http open() | ||
| CHUNK_SIZE = 64 * 1024 * 1024 # 64 MiB |
There was a problem hiding this comment.
Looks like it is now only used in http, so I guess no need to have it in the base class.
| # Needed for some providers, and http open() | |
| CHUNK_SIZE = 64 * 1024 * 1024 # 64 MiB |
There was a problem hiding this comment.
It's used in the open() call of _transfer_file()
Co-authored-by: Ruslan Kuprieiev <kupruser@gmail.com>
4 participants
All trees
Previously implemented
Implemented in this PR