Reduce S3 Calls (GSI-2490)#241
Merged
Merged
Conversation
TheByronHimes
marked this pull request as ready for review
July 8, 2026 21:00
Coverage Report for CI Build 29416503475Warning No base build found for commit Coverage: 93.664%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
…d improve error handling
TheByronHimes
force-pushed
the
feature/reduce_s3_calls_GSI-2490
branch
from
July 13, 2026 13:03
e5dd2b6 to
21a3e2d
Compare
mephenor
requested changes
Jul 14, 2026
mephenor
left a comment
Member
There was a problem hiding this comment.
Looks good, I just some additional places that could use slight improvement.
Do the existing tests not cover all the failure modes for the provider?
If so, should we extend them?
mephenor
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-evaluated every S3 provider operation and removed defensive existence checks where the main S3 call already fails with a translatable error.
Removed prechecks
delete_bucketandlist_all_object_idsrely onNoSuchBucketlist_parts,abort_multipart_upload, andcomplete_multipart_uploadrely onNoSuchUploadget_object_etag/get_object_size/get_object_metadatarely onhead_objectfailingOptimized
_assert_object_(not_)existsnow checks the object first and only checks the bus one call instead of twoKept (with comments explaining why)
create_bucket- S3 silently succeeds re-creating an owned bucket, so we let the app handle that errorinit_multipart_upload- to prevent duplicate uploadsdelete_object- so the application decides how to interpret missing objectscopy_objectdest check - to prevent overwriting things by accidentNote for near future
We should consider removing
MultipleActiveUploadsError. The built-in check for multiple active uploads doesn't give us much. The methods already target specific uploads viaupload_id, and if an application needs enforce upload exclusivity then that should be done application-side. That would mandate a major version bump though.Version bumped to
8.5.0