Skip to content

feat(storage): support delete source objects on compose#34665

Merged
cpriti-os merged 4 commits into
mainfrom
compose-delete-source-ruby
Jun 30, 2026
Merged

feat(storage): support delete source objects on compose#34665
cpriti-os merged 4 commits into
mainfrom
compose-delete-source-ruby

Conversation

@nidhiii-27

Copy link
Copy Markdown
Contributor

Updates compose file sample to support deleting source objects optionally. Fixes b/441557254

Updates compose file sample to support deleting source objects optionally. Fixes b/441557254

[Generated-by: AI]
@nidhiii-27 nidhiii-27 marked this pull request as ready for review June 25, 2026 14:34
@nidhiii-27 nidhiii-27 requested review from a team and yoshi-approver as code owners June 25, 2026 14:34
refute_nil bucket.file remote_file_name
end

it "compose_file with delete_source_objects" do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: For resiliency against edge cases, it would be awesome to add a test to cover how the client behaves if a compose operation fails midway (ex. due to a transient API error). Otherwise LGTM!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Co-authored by AI Agent

@@ -431,6 +431,26 @@ def mock_cipher.random_key
refute_nil bucket.file remote_file_name

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: What do you think about asserting that the source files remain intact in this default test block above? I looked at the previous PR that adds in the delete_source_objects param and I'm wondering if we can add an extra check to validate the delete_source_objects: false value doesn't accidentally delete the source files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Co-authored by AI Agent

Resolve comments in files_test.rb by adding assertions on source files and testing error scenarios.

[Generated-by: AI]
end

refute_nil bucket.file remote_file_name
refute_nil bucket.file file_1_name

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add a comment to explain the assertion here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Co-authored by AI Agent

assert_nil bucket.file file_2_name
end

it "compose_file with delete_source_objects failing does not delete source objects" do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks like a compose failure mock and not delete source object failure? Could you name it accordingly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Co-authored by AI Agent

Address reviewer comments in files_test.rb.

[Generated-by: AI]
@cpriti-os cpriti-os merged commit f7b92a9 into main Jun 30, 2026
18 of 20 checks passed
@cpriti-os cpriti-os deleted the compose-delete-source-ruby branch June 30, 2026 08:34
@github-actions github-actions Bot added the release-please:force-run To run release-please label Jun 30, 2026
@release-please release-please Bot removed the release-please:force-run To run release-please label Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants