Skip to content

feat(storage): Add method to disable checksums on read object#4993

Open
vsharonlynn wants to merge 7 commits intogoogleapis:mainfrom
vsharonlynn:feat/storage-disable-checksum
Open

feat(storage): Add method to disable checksums on read object#4993
vsharonlynn wants to merge 7 commits intogoogleapis:mainfrom
vsharonlynn:feat/storage-disable-checksum

Conversation

@vsharonlynn
Copy link
Contributor

For issue #4285.

@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Mar 12, 2026
@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.78%. Comparing base (ee6b7bb) to head (79e3754).

Files with missing lines Patch % Lines
src/storage/src/storage/read_object.rs 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4993      +/-   ##
==========================================
- Coverage   92.81%   92.78%   -0.03%     
==========================================
  Files         225      225              
  Lines        8665     8673       +8     
==========================================
+ Hits         8042     8047       +5     
- Misses        623      626       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@coryan coryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by, as I may not be available tomorrow.

@vsharonlynn vsharonlynn force-pushed the feat/storage-disable-checksum branch 2 times, most recently from 565014b to 1759962 Compare March 16, 2026 06:21
@vsharonlynn vsharonlynn force-pushed the feat/storage-disable-checksum branch from 1759962 to 79e3754 Compare March 16, 2026 06:38
@vsharonlynn vsharonlynn marked this pull request as ready for review March 16, 2026 07:09
@vsharonlynn vsharonlynn requested a review from a team as a code owner March 16, 2026 07:09
@vsharonlynn vsharonlynn dismissed joshuatants’s stale review March 16, 2026 07:10

Sorry, dismissing this because I can't see this after rebasing to latest main. PTAL at the latest version. Thank you.

/// println!("object contents={:?}", contents);
/// # Ok(()) }
/// ```
pub fn compute_crc32c_checksum(mut self, v: bool) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an inconsistency in naming here vs. compute_md5(). Consider naming this comptue_crc32c().

Comment on lines +173 to +176
self.options
.checksum
.crc32c
.get_or_insert_with(Default::default);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code path is untested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants