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

feat: add object io reader/writer to manager #2622

Closed
wants to merge 6 commits into from

Conversation

jobstoit
Copy link

@jobstoit jobstoit commented Apr 26, 2024

Hello I wanna contribute an io.Reader for the DownloadManager and io.WriteCloser for the UploadManager.
I recently created my own wrapper s3io and figured I could just as well contribute the ObjectReader and ObjectWriter to the s3 sdk. (resolving issue #2247)

Having an io.Reader and io.Write(Close)r can significantly optimise memory consumption in your applications and is therefor a nice to have.
I designed it to also work in chunks/parts concurrently similar to how the current downloader/uploader work

This is my first PR here so maybe you can help me with the conventions.

Let me know what you think and if you'd like me to finish this

@jobstoit jobstoit requested a review from a team as a code owner April 26, 2024 19:45
@jobstoit jobstoit changed the title feat: add io reader to downloadmanager and io writecloser to uploadma… feat: add io reader/writer to manager Apr 26, 2024
@jobstoit jobstoit changed the title feat: add io reader/writer to manager feat: add object io reader/writer to manager Apr 26, 2024
@lucix-aws
Copy link
Contributor

This is very useful software in its own right but it's not really the interface we want in the transfer manager.

We appreciate the effort but in the interest of saving you time I don't see us accepting any form of contribution to the s3 manager in the near future. In its current state it really needs a holistic redesign. We will most likely pursue releasing a new major version of the feature when that work takes place.

@jobstoit
Copy link
Author

Thank you for the response @lucix-aws. Hope the PR could give some inspiration on how to stream s3 objects concurrently. Best of luck with the redesign.

I'll update my s3io package likely too cause I like my implementation I did here better 😅.

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.

2 participants