[CZID-8699] Multipart uploads #87
Conversation
…mics into raboukhalil/multipart-uploads
| expiration: int = 3600, | ||
| sts_client: STSClient = Depends(get_sts_client), | ||
| ) -> MultipartUploadCredentials: | ||
| policy = { |
There was a problem hiding this comment.
I'm using the same policy as the one we currently use in the Rails app:
There was a problem hiding this comment.
Is there any dis/advantage to supporting both multipart file uploads and single-file uploads limited to 5gb? I think the main advantage to supporting both would be if there are some clients that have a hard time with the more complicated flow (ex: does the JS AWS client support multipart uploads?)
There was a problem hiding this comment.
Yes, the JS client supports multipart uploads according to this, but I remember Jerry patched that library but not sure for what exactly. But we could totally support both, I don't think it hurts
There was a problem hiding this comment.
I'm a huge fan of supporting One Right Way to do something w/this project. So I'm fine with merging this for now, and adding support back in for single-part-uploads if/when we run into problems with clients. 🤷♀️
There was a problem hiding this comment.
Sounds good!
robertaboukhalil
requested review from
jgadling,
morsecodist,
rzlim08 and
j-x-han
| @@ -89,7 +89,7 @@ async def test_invalid_fastq( | |||
| assert fileinfo["status"] == "FAILED" | |||
|
|
|||
|
|
|||
| # Test generating signed URLs for file upload | |||
There was a problem hiding this comment.
TODO at some point - it would be nice to have a test that does the whole flow:
- create a sequencing read
- generate a signed upload url to upload a sequencing read file
- Upload the file
- Run the "upload complete endpoint"
- Download the file
There was a problem hiding this comment.
Yes, I love that!
Currently, the entities service supports file uploads through 1 signed URL upload, which limits file sizes to 5GB.
As we discussed in our syncs recently, we need to support larger file uploads (the web app and CLI rely on multipart uploads). This should hopefully make it easy to switch to the NextGen upload token generation 😀