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

RFC: Robust Upload System #22

Merged
merged 5 commits into from
Feb 3, 2021
Merged

RFC: Robust Upload System #22

merged 5 commits into from
Feb 3, 2021

Conversation

jjnesbitt
Copy link
Member

@jjnesbitt jjnesbitt commented Jan 19, 2021

This is the long await "S3 API RFC". Sorry for the delay, but I believe this is in a good spot now.
Fixes #20

@jjnesbitt jjnesbitt requested review from waxlamp and JackWilb January 19, 2021 18:04
Copy link
Member

@JackWilb JackWilb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some preliminary thoughts.

I think that your second idea of tracking parts instead of the whole file as one is much more robust and gives us the features that we'd like (resuming, progress tracking, etc.). If we're all in agreement, it'd be great to see a second draft with that idea in more detail.

draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor requests to complete the RFC metadata, and some questions/opinions.

This actually looks about ready to go to me, once the open threads are resolved to everyone's satisfaction.

There is a lot to do for this feature, so after this is finalized, let's create an action plan and figure out how to divide up the work. Thanks @AlmightyYakob!

draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
draft/XXXX-robust-api/README.md Outdated Show resolved Hide resolved
@jjnesbitt jjnesbitt requested a review from waxlamp February 1, 2021 19:20
Copy link
Member

@JackWilb JackWilb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm 99% okay with this RFC. We should clean up the section regarding whether to store the ETags on the client or the server. I believe server was the preferred choice -- to allow resuming uploads -- but correct me if I'm wrong. If so, that'd mean removing the references to the client having to send all the ETags on finalize, too.

@jjnesbitt jjnesbitt requested a review from JackWilb February 2, 2021 16:55
Copy link
Member

@JackWilb JackWilb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this work, Jake!

@jjnesbitt jjnesbitt merged commit a088e25 into master Feb 3, 2021
@jjnesbitt jjnesbitt deleted the robust-api branch February 3, 2021 17:13
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.

Write new S3 upload API RFC
3 participants