Skip to content

Add support for custom SSH client on connect#2791

Merged
worksofliam merged 2 commits intomasterfrom
feature/custom-client
Jul 22, 2025
Merged

Add support for custom SSH client on connect#2791
worksofliam merged 2 commits intomasterfrom
feature/custom-client

Conversation

@SanjulaGanepola
Copy link
Copy Markdown
Member

@SanjulaGanepola SanjulaGanepola commented Jul 15, 2025

This PR adds support for a custom SSH client to be provided on connect.

This change is required for the itest CLI tool (IBM/vscode-ibmi-testing#137) where I am pulling in the Code for IBM i API itself as a dependency. I need to be able to provide a custom client that has the same APIs as Node SSH, but instead simply runs everything directly using spawn as the CLI tool can run directly on the IBM i. The custom client will look something like this: LocalSsh.ts

@SanjulaGanepola
Add support for custom SSH client on connect
c61aa54 
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
@SanjulaGanepola SanjulaGanepola temporarily deployed to testing_environment July 15, 2025 03:11 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 15, 2025 •
edited
Loading

👋 A new build is available for this PR based on 38d04f9.

@SanjulaGanepola SanjulaGanepola mentioned this pull request Jul 15, 2025
Merged
4 tasks
@worksofliam
Copy link
Copy Markdown
Member

worksofliam commented Jul 15, 2025

Before we merge this, I wonder if it's time to cleanup the connect method before we add more parameters.

@SanjulaGanepola @sebjulliand I am thinking we should change the parameter to accept to objects:

  1. The connection details as it is today
  2. Connection options, which would include the remaining parameters

Thoughts?

@sebjulliand
Copy link
Copy Markdown
Member

sebjulliand commented Jul 21, 2025

Before we merge this, I wonder if it's time to cleanup the connect method before we add more parameters.

@SanjulaGanepola @sebjulliand I am thinking we should change the parameter to accept to objects:

1. The connection details as it is today

2. Connection options, which would include the remaining parameters

Thoughts?

@worksofliam agreed. conectionDetails and options would be fine for the connect method (and allow more flexibility in the future).

@worksofliam
Copy link
Copy Markdown
Member

worksofliam commented Jul 21, 2025

@SanjulaGanepola is this something you want to do as part of this pull request?

@worksofliam
Update connect API to use new object for settings
38d04f9 
Signed-off-by: worksofliam <mrliamallan@live.co.uk>
@worksofliam worksofliam temporarily deployed to testing_environment July 21, 2025 18:39 — with GitHub Actions Inactive
@worksofliam worksofliam requested a review from sebjulliand July 21, 2025 18:41
@worksofliam
Copy link
Copy Markdown
Member

worksofliam commented Jul 21, 2025

@sebjulliand I made the change and the tests are looking good - it was simple and shouldn't change anything during runtime.

SanjulaGanepola
SanjulaGanepola commented Jul 21, 2025
View reviewed changes
Copy link
Copy Markdown
Member Author

@SanjulaGanepola SanjulaGanepola left a comment

Choose a reason for hiding this comment

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

@worksofliam Looks good. Thanks for updating that

@worksofliam worksofliam merged commit c980bfc into master Jul 22, 2025
5 checks passed
@sebjulliand sebjulliand deleted the feature/custom-client branch October 23, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@worksofliam worksofliam Awaiting requested review from worksofliam

@sebjulliand sebjulliand Awaiting requested review from sebjulliand

Assignees

@SanjulaGanepola SanjulaGanepola

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SanjulaGanepola @worksofliam @sebjulliand