fix(source-google-drive): dedupe files with identical paths to prevent file-transfer staging collision - #83345
fix(source-google-drive): dedupe files with identical paths to prevent file-transfer staging collision#83345dallasinman wants to merge 1 commit into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
|
Note Autopilot progressive rollouts are not enabled for the following modified connector(s):
This is a courtesy heads-up only — it does not block merge or fail any check. |
|
Deploy preview for airbyte-docs ready!
Deployed with vercel-action |
|
|
|
↪️ Triggering Reason: The branch now conflicts with |
|
…t staging path collision Co-Authored-By: dallas.inman <dallas.inman@airbyte.io>
f3a1b4d to
766c445
Compare
|
☑️ Conflict resolved and pushed (
|
|
I checked out Nothing was pushed. A fresh |
|
↪️ Triggering Reason: |
|
|
🟡 No Regression Detected — Fix Not Exercised (live test proposed, awaiting human approval)
Summary: Comparison regression tests pass cleanly with zero behavioral deltas, but the standard integration-test drive contains no duplicate-path files, so the dedupe code path was never exercised. Every currently-affected customer connection runs in a managed workspace whose source secrets the regression harness cannot retrieve (infrastructure limitation), so the fix could not be exercised via regression testing. A live prove-fix test (pin one affected connection to the pre-release + one sync) is proposed and awaiting human approval in Slack; this comment will be updated with the result if approved. 📋 Evidence log
🔬 Evidence plan & pre-flight checksProving criteria: on a drive with ≥2 files resolving to the same folder-relative path, target emits one record per path (newest kept) + "Dropping duplicate…" warning; control emits duplicates (the condition that crashes file-transfer destinations with Pre-flight: viability ✅ · safety ✅ · not breaking ✅ (patch bump, no schema/spec/state changes) · reversible ✅. Note: files are now buffered until folder traversal completes (memory/latency tradeoff on very large drives), acknowledged in the PR. Live test proposal (pending approval): pin one Tier-2, unpinned, US connection currently failing every sync with the exact staging-collision signature to Next steps
|
|
|
|
|
|
|
|
🙋 Escalated to #human-in-the-loop per Hands-Free AI Triage Project triage next step. Reason: |
|
Context from the author on the prove-fix escalation, so whoever picks this up doesn't re-derive it:
The secret-store gap the bot flagged is real and separate: Airbyte Hosted Data workspaces keep their source config secrets outside the GSM project the harness reads, so prove-fix will fail the same way for every such workspace. Worth its own issue against the harness rather than blocking this PR. |
|
What
In file transfer mode, two Drive files that resolve to the same folder-relative path are staged to the same local path (
/staging/files/<relative path>). The destination reads and then deletes each staged file (FileChunkTask.kt:81-107), so the second record referencing that path throwsjava.io.FileNotFoundExceptionand the destination exits 1 — killing the whole sync, permanently, on every attempt.Real customer impact (org
92102db1-f952-4087-895f-196a3daee3ef): zero successful syncs since 2026-07-31, 5/5 attempts of every hourly job failing on the same staged path, blocking their Context Store entirely. 433 other files transfer fine; two duplicate-named pairs break everything.This revives the fix from #79696 (open, conflicted) rebased onto current
master.How
SourceGoogleDriveStreamReader.get_matching_filesnow collects matching files keyed by their folder-relative URI instead of yielding immediately, keeps the most recently modified file on a collision, and logs a warning naming the dropped Drive file ID, the colliding path, and the retained ID:Dedupe is keyed on
path + name, not the basename, so identically-named files in different folders are unaffected.Review guide
source_google_drive/stream_reader.py— dedupe by URI, newest wins.unit_tests/test_reader.py— two Drive IDs, one filename, one emitted record.metadata.yaml/pyproject.toml/docs/integrations/sources/google-drive.md— 0.5.27 → 0.5.28.Two things worth a reviewer's opinion:
Set[str]of seen paths with first-wins semantics would preserve streaming at the cost of not being able to prefer the newest duplicate.FileChunkTaskarguably should fail soft on a missing staged file rather than aborting the sync.User Impact
Syncs no longer fail when a Drive contains multiple files with the same name in the same folder. Tradeoff: only one of a duplicate-named pair is synced (the most recently modified); previously neither was, because the sync failed outright.
Can this PR be safely reverted and rolled back?
Requested by Dallas Inman to unblock a customer evaluation.
Link to Devin session: https://app.devin.ai/sessions/f1935df080e0485e9f80f7c2c9c70290
Requested by: dallasinman