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

correction to send size with S3 request #1666

Closed
wants to merge 1 commit into from

Conversation

albertOPT
Copy link

when testing I was initially unable to send files to S3 without this additional change. I figured I should contribute back to the community as this was helpful for my project.

Copy link
Contributor

@mbenson mbenson left a comment

Choose a reason for hiding this comment

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

I don't understand... it appears that the call added at line 235 is already made at line 232.

@albertOPT
Copy link
Author

true, I believe 234 may have undone the initial setting of content length. I remember at the time getting error from S3 about missing content length before the change, we have been a build from this branch since the change was made. you're correct it would be cleaner to have removed 232 and moved down to 235 instead of duplicate lines.

@mbenson
Copy link
Contributor

mbenson commented Nov 14, 2018

The order of calls doesn't concern me either way, but reviewing the code of ObjectMetadata I don't see any reason to suppose that #setContentType() and #setContentLength() would impact one another in any fashion. Any chance you could double check that the relevant functionality is still broken for you using latest master?

@mbenson
Copy link
Contributor

mbenson commented Nov 16, 2018

I'm pretty sure this report must have been a duplicate of #1654 and am recommending it be closed.

@blafond blafond closed this Mar 8, 2019
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.

3 participants