Skip to content

feat(storage): Override storage class in writers to RAPID when rapid writes enabled for Pirlo Buckets#4889

Open
vipnydav wants to merge 2 commits into
masterfrom
set-rapid-class-for-pirlo
Open

feat(storage): Override storage class in writers to RAPID when rapid writes enabled for Pirlo Buckets#4889
vipnydav wants to merge 2 commits into
masterfrom
set-rapid-class-for-pirlo

Conversation

@vipnydav

Copy link
Copy Markdown
Member

Description

This PR updates the bucket handle to explicitly set the storage class to RAPID when creating objects (via CreateObject, CreateObjectChunkWriter, and CreateAppendableObjectWriter) if rapid writes are enabled for the Pirlo bucket. By default, the storage class is left empty to use the bucket's default, but when rapid writes are enabled (PirloStateRapidWritesEnabled), the intention to write to the RAPID class is now explicitly honored.

Link to the issue in case of a bug fix.

b/534575467

Testing details

  1. Manual - NA
  2. Unit tests - Added .
  3. Integration tests - Automated

Any backward incompatible change? If so, please explain.

No.

@vipnydav
vipnydav requested a review from a team as a code owner July 15, 2026 17:39
@vipnydav
vipnydav requested a review from kislaykishore July 15, 2026 17:39
@github-actions github-actions Bot added the remind-reviewers Auto remind reviewers in attention set for review post 24hrs of inactivity on PR. label Jul 15, 2026
@vipnydav vipnydav added execute-integration-tests Run only integration tests execute-integration-tests-on-zb To run E2E tests on zonal bucket. labels Jul 15, 2026

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

Copy link
Copy Markdown
Contributor

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 introduces logic to override the storage class to "RAPID" when rapid writes are enabled across various object creation methods (CreateObject, CreateObjectChunkWriter, and CreateAppendableObjectWriter), along with corresponding unit tests. The review feedback suggests avoiding direct mutation of the caller's request structs by using shallow copies instead, removing a redundant storage class override in CreateAppendableObjectWriter where it has no effect, and updating the unit tests to assert on the returned object attributes rather than the input request.

Comment thread internal/storage/bucket_handle.go
Comment thread internal/storage/bucket_handle.go
Comment thread internal/storage/bucket_handle.go Outdated
Comment thread internal/storage/bucket_handle_test.go Outdated
@vipnydav
vipnydav force-pushed the set-rapid-class-for-pirlo branch 2 times, most recently from adfc500 to f9d7314 Compare July 16, 2026 11:37
@vipnydav
vipnydav force-pushed the set-rapid-class-for-pirlo branch from f9d7314 to 40d086e Compare July 16, 2026 12:02
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.84%. Comparing base (d347c1c) to head (40d086e).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
internal/storage/bucket_handle.go 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4889      +/-   ##
==========================================
- Coverage   83.86%   83.84%   -0.03%     
==========================================
  Files         173      173              
  Lines       21068    21090      +22     
==========================================
+ Hits        17669    17683      +14     
- Misses       2741     2745       +4     
- Partials      658      662       +4     
Flag Coverage Δ
unittests 83.84% <50.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

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

Labels

execute-integration-tests Run only integration tests execute-integration-tests-on-zb To run E2E tests on zonal bucket. remind-reviewers Auto remind reviewers in attention set for review post 24hrs of inactivity on PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants