fix(drive): correct S3 presign region, push download status over socket - #360
Merged
Merged
Conversation
- generate_presigned_url signs offline, so it never gets the automatic region-redirect retry a real S3 call gets — signed against the client's default region, S3 rejects the URL outright with PermanentRedirect for any bucket outside that region (hit in prod: drive.frappe.cloud lives in ap-south-1, client defaulted to us-east-1). Detect and cache each bucket's real region via get_bucket_location (itself region-agnostic) and sign with a client that matches. - replace the 2s download_status poll with a single socket subscription: build_download_archive now publishes the terminal ready/failed state to the requesting user's realtime room, and the client just listens (with one immediate status check as a race guard for a build finishing before the listener attaches, and the existing timeout as a fallback if the event is ever dropped). - swap the manual "preparing" toast for frappe-ui's toast.promise — one loading toast that morphs into success/error in place, instead of hand-tracking an id across two separate toast() calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
generate_presigned_urlsigns offline and never gets boto3's automatic region-redirect retry that a real S3 call gets. Signed against the client's default region, S3 rejected the URL outright withPermanentRedirectfor any bucket outside that region — hit in prod, sincedrive.frappe.cloudlives inap-south-1and the client defaulted tous-east-1. Each bucket's real region is now detected viaget_bucket_location(itself region-agnostic) and cached, and presigning uses a client that matches.download_statuspoll with a single socket subscription:build_download_archivepublishes the terminalready/failedstate to the requesting user's realtime room, and the client just listens (with one immediate status check as a race guard for a build finishing before the listener attaches, plus the existing timeout as a fallback if the event is ever dropped).frappe-ui'stoast.promise— one loading toast that morphs into success/error in place instead of hand-tracking an id across separatetoast()calls.Builds on #359 (already merged) — this branch is off
develop.Test plan
ap-south-1bucket viabench console(get_bucket_location+ region-scoped presign)download_statuspolling in the network tab — only one socket event drives completion🤖 Generated with Claude Code