Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resurrect ssh multiplexing #4699

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Dec 6, 2023

- What I did
resurrect #2132 which has been reverted due to CI flakyness

- How I did it
git revert + sme tweaks to adjust to current codebase

- How to verify it

- Description for the changelog
Add multiplexing to ssh connection helper so commands that do multiple requests share the connection. Environment variables for opting out and controlling the persist time for even better connection sharing.

- A picture of a cute animal (not mandatory but encouraged)

@ndeloof ndeloof requested a review from thaJeztah as a code owner December 6, 2023 11:24
@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2023

Codecov Report

Merging #4699 (937954d) into master (cddd186) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4699      +/-   ##
==========================================
- Coverage   59.75%   59.72%   -0.04%     
==========================================
  Files         287      287              
  Lines       24821    24835      +14     
==========================================
  Hits        14832    14832              
- Misses       9103     9117      +14     
  Partials      886      886              

@thaJeztah
Copy link
Member

Some minor linting issues; one of them is probably to use 0o700 instead of 0700 (the new format with the o for octal)

59.95 cli/connhelper/connhelper.go:10: File is not `gofumpt`-ed (gofumpt)
59.95 	"strconv"
59.95 cli/connhelper/connhelper.go:86: File is not `gofumpt`-ed (gofumpt)
59.95 	if err := os.MkdirAll(config.Dir(), 0700); err != nil {
59.95 cli/connhelper/connhelper.go:89: File is not `gofumpt`-ed (gofumpt)
59.95 	sshFlags = append(sshFlags, "-o", "ControlMaster=auto", "-o", "ControlPath=" + config.Dir() + "/%r@%h:%p")
59.95 cli/connhelper/connhelper.go:103: File is not `gofumpt`-ed (gofumpt)
59.95 
59.95 cli/connhelper/connhelper.go:102: File is not `goimports`-ed (goimports)

@thaJeztah
Copy link
Member

Oh! And looks like you missed adding DCO sign-offs 😅

This reverts commit 2a08462.

Signed-off-by: Nicolas De Loof <[email protected]>
This reverts commit 29734b9.

Signed-off-by: Nicolas De Loof <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants