-
Notifications
You must be signed in to change notification settings - Fork 0
Az.Ssh Preview 2 #3
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
base: main
Are you sure you want to change the base?
Conversation
sshProcess.StartInfo.RedirectStandardError = true; | ||
sshProcess.Start(); | ||
|
||
bool sshSuccess = WaitSSHTunnelConnection(sshProcess); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a timeout for this connection wait? Can this wait be interrupted with Ctl+C?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it can be interrupted with a Ctrl+c, and in that case the function returns false and the cmdlet exits. I'm not totally sure if having a timeout is necessary since this function exits once the ssh process exits, and ssh has its own timeout. But I'll look into it and make sure.
|
||
if (SshLogsPrinted() || IsDebugMode()) | ||
{ | ||
var stderr = sshProcess.StandardError; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is stderr still valid after the process exits? Can stderr.Readline()
throw?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, stderr is still valid once the process exits. You can still read any messages that were sent to stderr in between the last time you read stderr and the moment the process exits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think stderr.Readline() can throw as long as stderr is valid. I can add a check to make sure stderr is not null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe stderr
is a StreamReader
, in which case it can throw a IOException
exception. Not sure if you want to catch it or let it propagate.
…ype. Co-authored-by: Paul Higinbotham <[email protected]>
Co-authored-by: Beisi Zhou <[email protected]>
Description
Checklist
CONTRIBUTING.md
and reviewed the following information:generation
branch.ChangeLog.md
file(s) appropriatelyChangeLog.md
file can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
## Upcoming Release
header in the past tense. Add changelog in description section if PR goes intogeneration
branch.ChangeLog.md
if no new release is required, such as fixing test case only.