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

Support ssh gateway when connecting from workspace explorer #89

Merged
merged 1 commit into from
Jul 24, 2023

Conversation

jeanp413
Copy link
Member

@jeanp413 jeanp413 commented Jul 22, 2023

Description

Support ssh gateway when connecting from workspace explorer

How to test

  1. Disable local ssh in configcat
  2. Connect using workspace explorer, it should work


let sshDest: SSHDestination;
let password: string | undefined;
if (await this.experimentsService.getUseLocalSSHProxy()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not be here a check that local ssh is initialized? i.e the logic should be similar to open URI.

Copy link
Member

@akosyakov akosyakov Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it is initialised above. I find it is quite confusing with so many paths. I think there should be only one function which hides the entires connectivity logic, i.e. we should not call initializeLocalSSH here and there, but only this function can. This function accepts the connection params as well as modes of opening a window, e.g. which folder and current or not, etc. All other clients start the workspace and then call to this function. It does not need to be the part of this PR, but would be nice to clean up.

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested both with Local SSH and SSH Gateway from the browser and view. And everything seems to work.

@jeanp413 jeanp413 merged commit 5088d1c into master Jul 24, 2023
1 check passed
@jeanp413 jeanp413 deleted the jp/widespread-horse branch July 24, 2023 20:16
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.

2 participants