Add CrcTransformer for checksum calculation#224
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #224 +/- ##
==========================================
+ Coverage 35.69% 36.49% +0.80%
==========================================
Files 127 128 +1
Lines 17583 17826 +243
Branches 17583 17826 +243
==========================================
+ Hits 6276 6506 +230
- Misses 10532 10540 +8
- Partials 775 780 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
St4NNi
left a comment
There was a problem hiding this comment.
Nice addition, as usual the whole transformer stuff is error prone. So there are a few things to look out for.
| let Ok((finished, should_flush)) = self.process_messages() else { | ||
| return Err(anyhow!("[HashingTransformer] Error processing messages")); | ||
| }; | ||
|
|
There was a problem hiding this comment.
I think there could be a race condition between receiving a file context and process_bytes. Maybe its best to assert this and fail if there is no file context when processing bytes.
There was a problem hiding this comment.
The fact that message and byte processing run asynchronously is a fundamental problem of the implementation of the Pithos library, not the transformer.
Of course, additional checks could be built in for this, but that would add a considerable degree of complexity that would not be relevant for our default use case.
St4NNi
left a comment
There was a problem hiding this comment.
This looks good now ! Approved !
This PR implements CRC checksum calculation support to enable data integrity validation for S3 uploads. By utilizing the crc-fast crate, the new CrcTransformer supports all known CRC-32 and CRC-64 variants.
Changes
Testing
All unit tests pass and verify correct checksum calculation for known input data across all three supported CRC algorithms.
Closes #218