perf(ssh,modal): bulk file sync via tar pipe and tar/base64 archive#7558
Closed
kshitijk4poor wants to merge 1 commit into
Closed
perf(ssh,modal): bulk file sync via tar pipe and tar/base64 archive#7558kshitijk4poor wants to merge 1 commit into
kshitijk4poor wants to merge 1 commit into
Conversation
a019a13 to
a9bd632
Compare
SSH: symlink-staging + tar -ch piped over SSH in a single TCP stream. Eliminates per-file scp round-trips. Handles timeout (kills both processes), SSH Popen failure (kills tar), and tar create failure. Modal: in-memory gzipped tar archive, base64-encoded, decoded+extracted in one exec call. Checks exit code and raises on failure. Both backends use shared helpers extracted into file_sync.py: - quoted_mkdir_command() — mirrors existing quoted_rm_command() - unique_parent_dirs() — deduplicates parent dirs from file pairs Migrates _ensure_remote_dirs to use the new helpers. 28 new tests (21 SSH + 7 Modal), all passing. Closes NousResearch#7465 Closes NousResearch#7467
a9bd632 to
0da9c7b
Compare
This was referenced Apr 11, 2026
5 tasks
Collaborator
|
Salvaged in #8014 — cherry-picked your commit onto current main with authorship preserved. Added a follow-up commit fixing a critical Modal ARG_MAX bug (the base64 payload was embedded in the command string, exceeding Modal SDK's 64KB exec-arg limit — rewrote to pipe through proc.stdin), plus minor cleanup (removed narrating comments, deduplicated Daytona helpers). Thank you for the contribution @kshitijk4poor! |
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
2 participants
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
Wire
bulk_upload_fninto SSH and Modal backends, matching the pattern established for Daytona in #7538. Eliminates per-file transfer overhead duringFileSyncManager.sync().Also extracts shared helpers into
file_sync.pyand migrates_ensure_remote_dirsto use them.Changes
Shared (
tools/environments/file_sync.py)quoted_mkdir_command()— mirrors existingquoted_rm_command()unique_parent_dirs()— deduplicates parent directories from(host, remote)pairsSSH (
tools/environments/ssh.py)_ssh_bulk_upload(): symlink-staging directory mirroring the remote layout, piped throughtar -ch | ssh tar xin a single TCP streammkdir -pcall_scp_upload)_ensure_remote_dirsto use shared helpersModal (
tools/environments/modal.py)_modal_bulk_upload(): builds an in-memory gzipped tar archive, base64-encodes it, decodes+extracts in oneexeccallexeccall total)Tests
test_ssh_bulk_upload.py— 21 tests across 4 classes: tar pipe mechanics (14), FileSyncManager wiring (1), shared helpers (6), edge cases (1)test_modal_bulk_upload.py— 7 tests: empty noop, tar archive contents, mkdir coverage, single-exec verification, exit code error, FileSyncManager wiring, timeoutBackend status
upload_files()tar -ch | ssh tar xtar+base64 | exec28 new tests, all passing. Existing file_sync, SSH, and Modal tests unaffected (72 passed, 11 skipped integration).
Supersedes #7560.
Closes #7465
Closes #7467