Skip to content

[AIRFLOW-1158] handle remaining bytes of the multipart upload#2337

Closed
stellah wants to merge 1 commit intoapache:masterfrom
stellah:s3_hook_remaining_multipart
Closed

[AIRFLOW-1158] handle remaining bytes of the multipart upload#2337
stellah wants to merge 1 commit intoapache:masterfrom
stellah:s3_hook_remaining_multipart

Conversation

@stellah
Copy link
Copy Markdown

@stellah stellah commented Jun 1, 2017

Dear Airflow maintainers,

This fixes multipart upload to S3 where the remaining bytes after multiples of 5*1024**3 bytes don't get uploaded.

JIRA

Tests

  • Please note that no tests were added as the existing S3_Hook
    doesn't have tests
  • However, this was tested locally and is being used in our production environment

@stellah
Copy link
Copy Markdown
Author

stellah commented Jun 1, 2017

This passed on all tests except for on Python 2.7 postgres. The fix doesn't have anything to do with this . I see that other PRs have this issue as well.

@bolkedebruin
Copy link
Copy Markdown
Contributor

Please add tests (absence of tests is no excuse for not adding them) and update your commit message per guidelines.

Btw I do think we have tests for s3. Didn't check though.

@saguziel
Copy link
Copy Markdown
Contributor

saguziel commented Jun 7, 2017

WRT tests, write tests that would break without this fix, and work with it.

@stellah
Copy link
Copy Markdown
Author

stellah commented Jun 8, 2017

I would need a test s3 bucket. Do you guys have one? I still don't see any s3 tests.

@r39132
Copy link
Copy Markdown
Contributor

r39132 commented Apr 29, 2018

@stellah This is a good fix. Can you add some tests and rebase?

@stellah
Copy link
Copy Markdown
Author

stellah commented May 1, 2018

@r39132 It looks like this fix is no longer needed, as you guys use boto3 upload_file now instead of FileChunkIO.

@r39132
Copy link
Copy Markdown
Contributor

r39132 commented May 2, 2018

@stellah Thx. I'll close this.

@asfgit asfgit closed this in 8c49e8d May 2, 2018
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.

4 participants