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

S3 implementation of Drive.putStream() missing required header #2

Closed
thecodemill opened this issue Nov 22, 2021 · 7 comments
Closed
Assignees
Labels
Type: Enhancement Improving an existing feature

Comments

@thecodemill
Copy link

thecodemill commented Nov 22, 2021

The S3 implementation of Drive.putStream() is currently broken.

S3 requires a ContentLength header and manually adding one fixes the issue. However, as we can't know the length of a readable stream, this results in "E_CANNOT_WRITE_FILE: Cannot write file at location" errors by default. It would be great if Drive could handle this automatically, eg. the approach outlined at aws/aws-sdk-go#122 (comment). Failing that, an update to the documentation to explain the limitation would be appreciated.

Thanks


References:

@leonardomarciano
Copy link

+1

@capoia
Copy link

capoia commented Dec 19, 2021

@thecodemill did you manage to solve this problem? is there any way to set the content length manually?

@thecodemill
Copy link
Author

@capoia No, in the end I made a change to our business logic to store the file size (amongst other metadata properties) at the time we receive the file. This size can then be referenced when streaming to S3. The process was already writing a db record, so adding a few extra columns actually turned out to be a bonus 👍

@capoia
Copy link

capoia commented Dec 22, 2021

@capoia No, in the end I made a change to our business logic to store the file size (amongst other metadata properties) at the time we receive the file. This size can then be referenced when streaming to S3. The process was already writing a db record, so adding a few extra columns actually turned out to be a bonus +1

But how do you upload (using the Drive that adonis provides) the file size?
I know using other libraries, but using Drive.putStream, how do I send the content-length?

@thecodemill
Copy link
Author

thecodemill commented Dec 22, 2021 via email

@capoia
Copy link

capoia commented Dec 23, 2021

Third argument of putStream() accepts an array of additional headers. Just send it as a “Content-Length” header.

Sent from my iPhone
On 22 Dec 2021, at 21:02, capoia @.***> wrote:  @capoia No, in the end I made a change to our business logic to store the file size (amongst other metadata properties) at the time we receive the file. This size can then be referenced when streaming to S3. The process was already writing a db record, so adding a few extra columns actually turned out to be a bonus +1 But how do you upload (using the Drive that adonis provides) the file size? I know using other libraries, but using Drive.putStream, how do I send the content-length? — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

I don't know if this has changed recently but the third option is of type ContentHeaders, and it doesn't include contentlenght.
image

@thetutlage
Copy link
Member

The ContentHeaders type has a list of whitelisted properties that is applicable across all the drivers. However, you can still send additional headers accepted by s3.

{
  "Content-Length": theValue
}

@thecodemill Sorry, I haven't been able to pay much attention to this issue. I will go through your shared links and will act accordingly

@thetutlage thetutlage self-assigned this Dec 23, 2021
@thetutlage thetutlage added the Type: Enhancement Improving an existing feature label Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Improving an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants