-
Notifications
You must be signed in to change notification settings - Fork 299
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
Docker command migration #1281
Docker command migration #1281
Conversation
// this always has length | ||
if (lines.slice(-1)[0] === '') lines.pop(); | ||
|
||
lines.forEach((line) => log.info(line)); |
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.
Shouldn't all this continue inside a try catch?
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.
Yeah nice spot, the stdout.split() could throw, I'll fix.
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.
This highlighted an error I had in runCommand, stdout and stderr as allways returned as strings from child_process execFile. I've fixed this up (and tests). Now the above code will never throw. Howver, for safety, have wrapped in a try / catch
What this pull does
*. Uses the more modern
findmnt
instead of mount*. Added a bunch of notes so in 6 months time I remember how things work.
*. Removes the df command - not needed anymore with findmnt.
*. Log output if the module is run via node directly.
@XK4MiLX - Can you please check the deviceHelper stuff, I'm pretty sure this is correct but want to make certain.
I haven't run this on any nodes yet - will do that this afternoon.