-
Notifications
You must be signed in to change notification settings - Fork 38
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
Allow disabling upload checksums while using upload review #421
Conversation
Some clients (Mountpoint) might want to disable actually sending upload checksums when talking to S3 endpoints that don't support the additional checksums feature. But they still want to use the upload review callback to at least check that the parts sent matched the checksums they expect. This change makes it possible to set a checksum location of AWS_SCL_NONE while still computing checksums for each upload part. The upload review callback will be invoked as normal, but the checksums won't be included in the requests sent to S3. Signed-off-by: James Bornholt <bornholt@amazon.com>
Signed-off-by: James Bornholt <bornholt@amazon.com>
Before we were relying on the chunk stream to reach the end of its inner checksum stream and then immediately destroy the checksum stream, triggering the checksum finalizer. We need to do the same thing in the non-chunk-stream case. Signed-off-by: James Bornholt <bornholt@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for creating the PR. I have made a small change to finalize error handling since now it can finalize in the read_callback. LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trivial stuff.
fix&ship. (if you don't mind, I can fix it for you)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #421 +/- ##
==========================================
- Coverage 89.63% 89.52% -0.12%
==========================================
Files 20 20
Lines 5992 6012 +20
==========================================
+ Hits 5371 5382 +11
- Misses 621 630 +9
|
Some clients (Mountpoint) might want to disable actually sending upload
checksums when talking to S3 endpoints that don't support the additional
checksums feature. But they still want to use the upload review callback
to at least check that the parts sent matched the checksums they expect,
to detect corruption while parts were sitting in CRT memory before being sent.
This change makes it possible to set a checksum location of AWS_SCL_NONE
while still computing checksums for each upload part. The upload review
callback will be invoked as normal, including per-part checksums, but the
checksums won't be included in the requests sent to S3.
(My C is pretty rusty these days, so, uh, feedback welcome)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.