-
Notifications
You must be signed in to change notification settings - Fork 387
feat(listBuckets): Add support for returning partial success #2678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implements the optional parameter `returnPartialSuccess`. If set to `true` and the API response contains an `unreachable` array, the function will return a successful callback (`err: null`) and pass the `unreachable` array as the 5th argument. This allows users to process the available buckets without the entire operation failing on soft errors.
New sample for partial success in ListBuckets (storage_list_buckets_partial_success)
|
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
|
Warning: This pull request is touching the following templated files:
|
src/bucket.ts
Outdated
| * that the API failed to retrieve (unreachable) due to partial failure. | ||
| * Consumers must check this flag before accessing other properties. | ||
| */ | ||
| unreachable?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can make this unreachable: boolean. No need for it to be optional as a bucket can only be in 2 states.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you mentioned, there's no need to declare optional for unreachable, as it can cause some property errors. To fix this, want to initialize unreachable: boolean = false (which may show a warning) or simply unreachable = false (which won't show a warning).
src/storage.ts
Outdated
| const buckets = itemsArray.map((bucket: BucketMetadata) => { | ||
| const bucketInstance = this.bucket(bucket.id!); | ||
| bucketInstance.metadata = bucket; | ||
| bucketInstance.unreachable = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you make unreachable not optional, we can remove this as it should default to false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will initialize unreachable = false. This will set the default value to false; otherwise, unreachable would remain undefined here.
This reverts commit 10a7e87.
Description
This change implements the
returnPartialSuccessoption for thegetBuckets(orListBuckets) method to handle scenarios where the external API returns a successful response but includes anunreachablelist of resources.returnPartialSuccessis added to theGetBucketsRequestinterface.true: IfreturnPartialSuccessis set totrueand the API response containsresp.unreachableitems, the method will execute the callback successfully (witherr: null) and pass the array of unreachable bucket IDs as the fifth argument to the callback function.false(or not set) and unreachable items are present, the function follows the existing failure flow (or implicitly returns success, depending on the previous behavior).returnPartialSuccessparameter is correctly included in the query string (qs) sent to the API endpoint, as it is a known API parameter.Impact
This change significantly improves the robustness of the
ListBucketsoperation. Users can now choose to tolerate minor, non-critical failures (like temporary unavailability of a few regions) and successfully retrieve and process the majority of available buckets, preventing a complete application failure.Testing
returnPartialSuccess: trueis set and the API returns anunreachablearray.unreachable) is strictly optional (?) and is only provided when explicitly enabled via the new option.Additional Information
resp.unreachableproperty on a successful HTTP status code.unreachable) was chosen to avoid modifying the primary result type (Bucket[]) and to maintain compatibility with existing consumers who expect the 4-argument signature.Checklist
Fixes # 🦕