API upload object without using tmp file #4848
Conversation
Multipart request read content directly to block adapter. The Go implementation use local temporary file to enable read from content.
nopcoder
added
area/API
There was a problem hiding this comment.
Neat!
I think that this can change behaviour with some S3 "compatible" storage backends, since we're no longer uploading data with a length. We might need to add an option to turn this off.
Also, the current implementation will be dangerous if we ever add another part to UploadObject in swagger.yml. Let's add a comment on its definition not to do that!
pkg/api/controller.go
Outdated
| if !strings.HasPrefix(mt, "multipart/") { | ||
| writeError(w, r, http.StatusInternalServerError, http.ErrNotMultipart) | ||
| return | ||
| } |
There was a problem hiding this comment.
Not sure this works for any multipart Content-Type, perhaps we should limit ourselves to a list of known types? (Package http probably limits itself in some way, so if we do the same would we be safe?)
There was a problem hiding this comment.
Good point - I'll limit it to "multipart/form-data". The Go's request implementation support form-data and optional mixed.
There was a problem hiding this comment.
Also, the current implementation will be dangerous if we ever add another part to UploadObject in swagger.yml. Let's add a comment on its definition not to do that!
Currently we added the information in the swagger description: Only a single file per upload which must be named "content".
| writeError(w, r, http.StatusInternalServerError, err) | ||
| return | ||
| } | ||
| contentUploaded = true |
There was a problem hiding this comment.
Why not break here? Nothing remains that could be useful (I think).
There was a problem hiding this comment.
Will fix - wanted to complete the body processing - but I see that Close will do it for me!
The the backend will get the size as the s3manager upload operation works with a single put or multi-part upload. |
Fix #4849
Multipart request read content directly to block adapter.
The issue is that the Go implementation uses local temporary file when accessing content size >32MB.
In order to support reading the data directly in this case we stream the request data into the adapter's
Putrequest without specify the content size.To support Put with reader without size (stream), updated the S3 to use of s3manager upload in case the size == -1.
All other adapters ignore the size in the request to upload.
Tested manually with file size ~170MB