[RAPTOR-20129] fix(filesapi): send zip-path overwrite in the multipart form - #898
Conversation
|
🎫 Jira: |
Code OwnershipCli Maintainers
Review requested from the teams above. Labels will be removed automatically upon approval. |
|
/approve-smoke-tests |
|
🔐 Fork PR smoke tests triggered by @wojtekwdr What happens next:
|
|
🔐 Fork smoke tests started by maintainer ⏳ Security scans passed. Running smoke tests... Commit: |
|
✅ All smoke tests passed! (Fork PR) ✅ Security Scan: success |
…t form The Files API reads overwrite from the form, so a query-only REPLACE was accepted and ignored and every re-uploaded path came back as a renamed duplicate. Send the field ahead of the file part and keep the query copy until the contract says which location is authoritative.
8787c8e to
f83f3e3
Compare
adamalpi
left a comment
adamalpi
left a comment
There was a problem hiding this comment.
Fix is correct and the tests are thorough. Two inline notes, neither blocking: the explanatory comment states a rule the same function disproves, and the new parameter is swappable with the one beside it.
|
/approve-smoke-tests |
|
🔐 Fork PR smoke tests triggered by @ajalon1 What happens next:
|
|
🔐 Fork smoke tests started by maintainer ⏳ Security scans passed. Running smoke tests... Commit: |
|
❌ Some smoke tests failed. (Fork PR) ✅ Security Scan: success |
|
❌ Some smoke tests failed. (Fork PR) ✅ Security Scan: success |
…drop the dead query argument
|
/approve-smoke-tests |
|
🔐 Fork PR smoke tests triggered by @wojtekwdr What happens next:
|
|
🔐 Fork smoke tests started by maintainer ⏳ Security scans passed. Running smoke tests... Commit: |
|
✅ All smoke tests passed! (Fork PR) ✅ Security Scan: success |
datarobot-pr-review-router
Bot
removed
00 - Ready for Review
Needs Review: CLI Maintainers
labels
3 participants
RATIONALE
dr artifact code syncanddr workload upupload throughPOST /files/{id}/fromFile/whenever a change set exceeds 20 files or 50 MB. The overwrite mode went only in the query string, which the Files API accepts and ignores, so the server's rename default applied: every existing path came back asname (2).extholding the new bytes while the original kept the old ones, and an image built from that version ran stale code. Reproduced on staging: a 25-file re-sync produced a 53-file version with 25 duplicates. The Terraform provider carried the same code and fixed it the same way.CHANGES
UploadFromZipExistingsendsoverwritein the form and keeps the query copy until the API documents which location is authoritative.TESTING
Staging, fixed binary, 25 modified files:
dr artifact code syncre-sync gives 28 files and no duplicates (53 with 25 duplicates before), and adr workload upre-deploy gives 27 files and no duplicates, with the server checksum of the edited file matching the local one.task linton all three GOOS legs andtask testare green.RELATED
PR #867 bundles the same fix with unrelated integrity work and is currently red; this is the standalone extract so it can ship on its own, and #867 can rebase onto it.
useArchiveContentsstaying in the query is RAPTOR-19915.Note
Medium Risk
Changes deploy/sync upload behavior for large change sets; incorrect framing could break uploads, but scope is limited to the zip fromFile path and is covered by new tests.
Overview
Fixes catalog zip uploads (
dr artifact code syncand large-changedr workload uppaths) so existing files are replaced instead of duplicated. The Files API was ignoringoverwritewhen it was only on the query string; the server’s rename default producedname (2).extcopies and left old bytes in place, so built images could run stale code.UploadFromZipExistingnow sendsoverwriteas a multipart form field before the zip part (query param kept for compatibility). Streaming multipart framing was extended to emit optional form fields with correct Content-Length; stage uploads pass no extra fields.Tests assert form field order, query copy, payload integrity on large bodies, and that new-catalog zip uploads still send only the file part.
Reviewed by Cursor Bugbot for commit 8787c8e. Configure here.