Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple changes #339

Merged
merged 10 commits into from
May 27, 2024
Merged

Multiple changes #339

merged 10 commits into from
May 27, 2024

Conversation

terricain
Copy link
Owner

No description provided.

kyboi and others added 6 commits April 29, 2024 17:54
…r config usage

* Improved typing somewhat albeit is no subsitiute for those full type generation scripts.
* Fixed concurrency issue in _download_part where writing to seekable async file-like objects, the seek and write operations were not guaranteed to happen sequentially. An asyncio.Lock solves this.
* Added non-seekable IO support to download_fileobject. There are some issues here in terms of resource consumption/utilisation. The coroutines to download chunks can happen in any order which means
  chunks at the end of a file could be downloaded first, and thus must be kept in memory until all their preceding chunks have been downloaded, this is fixable by moving the code to something similar
  to how upload part works. The other thing is unrelated to seekable IO and more that downloading a 1GiB file with an 8MB chunksize results in 128 tasks being gathered, isn't that much of an issue but
  if the chunksize is lowered or the file is larger then the event loop could get bogged down with lots of tasks.

Performance on average downloading a 1GiB file on a somewhat unutilised network is around 16 seconds which matches the original S3 transfer speeds.
@terricain terricain marked this pull request as ready for review May 27, 2024 15:57
@terricain terricain merged commit 7a0ea17 into main May 27, 2024
4 checks passed
@terricain terricain deleted the all_the_fixes branch May 27, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants