Skip to content

feat(storage): remove deprecated legacy checksum options completely - #16279

Open
v-pratap wants to merge 15 commits into
googleapis:mainfrom
v-pratap:remove-legacy-checksum-options
Open

feat(storage): remove deprecated legacy checksum options completely#16279
v-pratap wants to merge 15 commits into
googleapis:mainfrom
v-pratap:remove-legacy-checksum-options

Conversation

@v-pratap

Copy link
Copy Markdown
Contributor

This PR removes the deprecated DisableMD5Hash and DisableCrc32cChecksum completely from the codebase as requested for the next major release. This includes:

  • Removing deprecated options from public headers (hashing_options.h).
  • Removing legacy fallback parsing logic.
  • Migrating all internal uses (tests, streambuf, validators) to use UploadChecksumValidationOption and DownloadChecksumValidationOption.

This also includes the missing checksum options migrations from parallel_upload.h and the benchmarks.

@v-pratap
v-pratap requested review from a team as code owners July 28, 2026 08:41
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Jul 28, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request removes the deprecated DisableMD5Hash and DisableCrc32cChecksum options, replacing them with UploadChecksumValidationOption and DownloadChecksumValidationOption across the codebase, including tests and benchmarks. The review feedback highlights several style guide violations, such as lines exceeding the 80-character limit, excessive blank lines, and a broken Doxygen comment block. Additionally, it is recommended to factor out a repeated nested ternary expression in throughput_experiment.cc into a helper function to reduce duplication and improve readability.

Comment thread google/cloud/storage/benchmarks/throughput_experiment.cc Outdated
Comment thread google/cloud/storage/internal/checksum_helpers.h
Comment thread google/cloud/storage/internal/checksum_helpers.h
Comment thread google/cloud/storage/internal/hash_function.cc Outdated
Comment thread google/cloud/storage/internal/hash_function.cc Outdated
Comment thread google/cloud/storage/internal/object_write_streambuf_test.cc
Comment thread google/cloud/storage/internal/object_requests_test.cc
Comment thread google/cloud/storage/client_object_test.cc
Comment thread google/cloud/storage/hashing_options.h
Comment thread google/cloud/storage/hashing_options.h
@v-pratap v-pratap added the do not review Indicates a PR is not ready for review label Jul 28, 2026
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.87324% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.30%. Comparing base (f8a0cac) to head (f12e48c).

Files with missing lines Patch % Lines
.../cloud/storage/benchmarks/throughput_experiment.cc 0.00% 12 Missing ⚠️
.../cloud/storage/internal/hash_function_impl_test.cc 91.66% 1 Missing ⚠️
...ogle/cloud/storage/internal/hash_validator_test.cc 83.33% 1 Missing ⚠️
google/cloud/storage/testing/upload_hash_cases.cc 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16279      +/-   ##
==========================================
- Coverage   92.30%   92.30%   -0.01%     
==========================================
  Files        2223     2223              
  Lines      208086   207899     -187     
==========================================
- Hits       192074   191893     -181     
+ Misses      16012    16006       -6     

☔ View full report in Codecov by Harness.
📢 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.

@v-pratap
v-pratap force-pushed the remove-legacy-checksum-options branch from ff1c503 to 92a3ed7 Compare July 29, 2026 08:46
@v-pratap v-pratap removed the do not review Indicates a PR is not ready for review label Jul 29, 2026
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.

1 participant