- Notifications
You must be signed in to change notification settings - Fork914
chore: add parent PID to coder ssh log file name#16080
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Part of bringing coder ssh to parity with coder vscodessh is associatingthe log files with a particular parent process (in this case, the sshprocess that spawned the coder CLI via ProxyCommand). coder vscodesshnamed log files using the parent PID, but coder ssh is missing this. Addthe parent PID to the log file name when used in stdio mode so thatthe VS Code extension will be able to identify the correct log file.
| ) | ||
| if stdio { |
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.
InsetStatsCallback, we use the parent pid as the filename regardless of whether or not we're in stdio mode:https://github.com/coder/coder/pull/16078/files#diff-e288cc849ca774d47294f4f6c71c4ec171f6e0b15491e7220a0144dbfc1a43bcR1213
Should we just include it in either case here as well? Trying to get a handle on how these files relate I suppose.
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.
For now I think this is good for compatibility though.
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 was thinking we only want it for stdio mode because including the parent PID is only meaningful whencoder ssh is invoked viaProxyCommand (which would use--stdio). Happy to change this if you prefer to be consistent, though.
There isn't a use case where we would use--network-info-dir without--stdio, but I thought I should be consistent and support it in either case. I could make it return an error when--stdio is not also specified.
ec6645b intocoder:mainUh oh!
There was an error while loading.Please reload this page.
Part of bringing
coder sshto parity withcoder vscodesshis associating the log files with a particular parent process (in this case, the ssh process that spawned the coder CLI viaProxyCommand).coder vscodesshnamed log files using the parent PID, but coder ssh is missing this. Add the parent PID to the log file name when used in stdio mode so that the VS Code extension will be able to identify the correct log file.See also#16078.