feat(storage): support delete source objects on compose#34665
Conversation
Updates compose file sample to support deleting source objects optionally. Fixes b/441557254 [Generated-by: AI]
[Generated-by: AI]
| refute_nil bucket.file remote_file_name | ||
| end | ||
|
|
||
| it "compose_file with delete_source_objects" do |
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
Done
Co-authored by AI Agent
| @@ -431,6 +431,26 @@ def mock_cipher.random_key | |||
| refute_nil bucket.file remote_file_name | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Add a comment to explain the assertion here.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
This looks like a compose failure mock and not delete source object failure? Could you name it accordingly
There was a problem hiding this comment.
Done
Co-authored by AI Agent
Address reviewer comments in files_test.rb. [Generated-by: AI]
Updates compose file sample to support deleting source objects optionally. Fixes b/441557254