-
Notifications
You must be signed in to change notification settings - Fork 294
Update pip.js #356
Update pip.js #356
Conversation
Add a `keyFile` option, so you could replace the default keyfile for one that is not `id_rds` This change was made to make possible a work arround for serverless#272 But it mey also be a interesting feature.
| dockerCmd.push( | ||
| '-v', | ||
| `${process.env.HOME}/.ssh/id_rsa:/root/.ssh/id_rsa:z`, | ||
| `${process.env.HOME}/.ssh/${options.keyFile || 'id_rsa'}:/root/.ssh/id_rsa:z`, |
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 think it would be more flexible if keyFile was a full path to a file:
| `${process.env.HOME}/.ssh/${options.keyFile || 'id_rsa'}:/root/.ssh/id_rsa:z`, | |
| options.keyFile ? `${options.keyFile}:/root/.ssh/id_rsa:z`: `${process.env.HOME}/.ssh/id_rsa:/root/.ssh/id_rsa:z`, |
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.
Or maybe search on absolute path. then current path, then .ssh.
What do you think?
Too much complex?
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.
if you want to implement what you suggested(with test coverage 😉) I'm cool with that. Definitely more complex tho.
|
Hi @wviana, are you still interested in completing this PR? |
|
@bsamuel-ui hi there. Yes, cause I do change this file on my node_modules till today. Here on mine I'm just changing to load the entire .ssh folder instead of choosing a file. |
|
Hey @wviana - it's been a long time since this PR was proposed. I'm going to close it, if you feel like the issue is valid, please open a new issue or a new PR against the latest main branch. Thanks 🙇 |
Add a
keyFileoption, so you could replace the default keyfile for one that is notid_rdsThis change was made to make possible a work arround for #272
But it may also be a interesting feature.