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

Respect checksum header over setting #474

Merged
merged 82 commits into from
Dec 11, 2024

Conversation

TingDaoK
Copy link
Contributor

@TingDaoK TingDaoK commented Dec 10, 2024

Issue #, if available:

  • we allow user to set checksum config from either the meta_request_options or implicitly from the header of the http request.
  • The config from the meta_request_options previously takes precedence before this PR.
  • If we keep the same trailing checksum config from meta_request_options for both multipart and single part upload and full object checksum from header
    • in the case of single part upload, we will have both trailer checksum and the header checksum, which will lead to an error
    • in the case of multipart upload, the parts level checksum will be added, and the full object checksum will be applied to the complete MPU, which will be the expected behavior.
  • In order to make it consistent, we now takes the header config over the meta_request_options config. So that we will ignore any settings from the meta request options and prefer the implicitly one from the header and make sure they land in the expected behavior.

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Base automatically changed from checksum-callback to main December 10, 2024 22:56
@TingDaoK TingDaoK marked this pull request as ready for review December 10, 2024 23:36
@TingDaoK TingDaoK merged commit 33b8cd0 into main Dec 11, 2024
34 checks passed
@TingDaoK TingDaoK deleted the respect-checksum-header-over-setting branch December 11, 2024 22:27
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.

2 participants