FEATURE: Direct S3 multipart uploads for backups (PR #14736)

WIP, working but very hacky with lots of copy/paste

GitHub

This is some of the jankyness mentioned in the PR description that I hope to address in a subsequent PR.

This is some of the jankyness mentioned in the PR description that I hope to address in a subsequent PR.

This is some of the jankyness mentioned in the PR description that I hope to address in a subsequent PR.

The title of this pull request changed from “DEV: WIP direct uppy backup uploads” to "FEATURE: Direct S3 multipart uploads for backups

I think it would be better to have a specific class for that label like backup-uploaded and format it with CSS instead of space character

I just removed the space from both templates, no class is needed because d-icon already has a margin

I think that both regexes should use \A and \z instead of ^ and $. I don’t think that anything bad can happen here, but this is a good habit as we already had issues from HackerOne where people could exploit our code by using multilines.

store_for_upload_type and multipart_store methods are almost the same, so you think it would be worth to extract them?

Is it possible to stub that method on spec level instead?

Maybe for 4 arguments we should use named parameters?

I’d like to keep this the same in this PR and change in a separate one, it’s just a copy+paste of the existing regex and I want to clean up this duplication in another PR anyway. Also changing the regex is scary to me so it’s probably best to isolate :slight_smile:

Yes fair enough

Yes you’re right will just use the ExternalUploadManager one in the controller

We actually do this same thing in a few places, I think it’s fine:

That way the spec can be more realistic