-
Notifications
You must be signed in to change notification settings - Fork 707
feat(service/s3): support crc64nvme algorithm #6815
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
Conversation
Signed-off-by: Ruihang Xia <[email protected]>
|
This crate seems pretty great |
core/Cargo.toml
Outdated
| "dispatcher", | ||
| ] } | ||
| # for services-s3 | ||
| crc32c = { version = "0.6.6", optional = true } |
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.
Should we just replace crc32c?
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.
done in e1082c7
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
|
Do we still want this after checksum has been implemented? I can imagine that we will deprecate all settings at the S3 level. |
# Conflicts: # core/Cargo.lock # core/Cargo.toml # core/services/s3/src/backend.rs # core/services/s3/src/config.rs
Makes sense. Should we move the overall development of checksum to the dedicated layer proposed in #6817 ? If so I can close this PR and continue on the layer |
Yes and thank you! |
Which issue does this PR close?
Rationale for this change
Support
crc64nvmeagain, withcrc-fast. It's the successor ofcrc64fast-nvme. Related discussion awesomized/crc64fast-nvme#13 (comment)What changes are included in this PR?
Support
crc64nvmechecksum algorithm for s3Are there any user-facing changes?
Yes, a new feature