Repository navigation
ssh: create the release folder before uploading the tunnel binaries - #7141
Draft
abdelrahmanomar1515 wants to merge 3 commits into
Draft
abdelrahmanomar1515 wants to merge 3 commits into
abdelrahmanomar1515 wants to merge 3 commits into
Conversation
Collaborator
Integration test reportCommit: 689064c
Top 3 slowest tests (at least 2 minutes):
|
import-file reports a missing parent folder with a 404 only after the whole archive has been sent. On the first upload of every new version, CreateParentDirectories then created the folder and sent the archive again, so each ~17 MB archive was uploaded twice and half of the import-file calls failed. Create the release folder with mkdirs, which succeeds when it already exists, and upload once. Co-authored-by: Isaac <no-reply@databricks.com>
abdelrahmanomar1515
force-pushed
the
ssh-workspace-import-releases
branch
from
October 21, 2026 15:08
ddaef3c to
2c6d8d4
Compare
abdelrahmanomar1515
changed the title
|
|
||
| // import-file reports a missing parent folder only after receiving the whole archive, so | ||
| // create the folder up front instead of letting CreateParentDirectories upload it twice. | ||
| err = workspaceFiler.Mkdir(ctx, remoteSubFolder) |
Member
There was a problem hiding this comment.
consider adding/modifying an existing acceptance test to record requests (that would show only a single /import-file call now instead of 2 before)
Author
There was a problem hiding this comment.
Done in b288cfb: ssh/connect-serverless-feature-disabled now prints the import-file requests, one per architecture. Without the mkdirs it records four, because each archive gets a 404 and is sent again.
- connect-serverless-feature-disabled now prints the import-file requests, showing one upload per architecture. - Keep CreateParentDirectories as the fallback for a 404 after the folder was created, and drop the comment. - The unit test reads status codes through the test server's ResponseCallback instead of copying the import-file handler. Co-authored-by: Isaac <no-reply@databricks.com>
janniklasrose
approved these changes
Oct 21, 2026
Contributor
|
Commit: Regression test reportSelected 1 changed tests (limit=20, 0 not selected) 1 tests: regression=1 unreleased=0 coverage=0 failing=0
TestAccept/ssh/connect-serverless-feature-disabled/DATABRICKS_BUNDLE_ENGINE=direct: branch ✅ | main (bef6321) ❌ | latest (v1.21.0) ❌main (bef6321): latest (v1.21.0): |
Run mkdirs before getting the release, so a failed mkdirs doesn't leave a started GitHub download open. Cover a dev re-upload into existing folders in the unit test, keeping each case's version in one place, and mention the upload check in the RecordRequests comment of connect-serverless-feature-disabled. Co-authored-by: Isaac <no-reply@databricks.com>
This branch has not been deployed
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
3 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.
Changes
ssh connectnow creates each release folder (<version>/databricks_cli_<version>_linux_<arch>/) withmkdirsbefore uploading the archive toimport-file.mkdirssucceeds when the folder already exists.CreateParentDirectoriesstays as the fallback if the upload still gets a 404.Why
import-filereports a missing parent folder with a 404 only after the whole archive has been sent. On the first upload of every new version, the filer then created the folder and sent the archive again. Every archive (~17 MB) was therefore uploaded twice, and half of theimport-filecalls showed up as failures.The upload stays on
import-file. Moving it to/workspace/importwas considered, butformat=AUTOimports a zip as a new folder that can't be overwritten. Overwriting is needed for dev builds and for concurrent connects. It also places an archive with a single top-level file next to that folder instead of inside it.Tests
ssh/connect-serverless-feature-disablednow records theimport-filerequests: one per architecture. Without the change it records four.TestUploadReleasesSendsEachArchiveOnce: a fresh release upload, and a dev re-upload into existing folders, both send oneimport-filerequest per architecture. Without the change the fresh upload records the reported pattern:amd64 404,amd64 200,arm64 404,arm64 200.import-file 404 → mkdirs 200 → import-file 200. The new one makesmkdirs 200 → import-file 200per architecture, with the same extracted layout../task lint-goand the ssh unit tests pass locally.This pull request and its description were written by Isaac.