remove shell-quote and switch to execsync #23
Conversation
| cwd: componentPath, | ||
| encoding: 'utf-8', | ||
| }); | ||
| const stdout = execSync(config.buildCommand, { |
There was a problem hiding this comment.
Might be worth noting that if there is any stderr from a build execution that succeeds (return code of 0), that stderr will be piped to process stderr, but I don't know if it will make it into the logs. Probably not a big deal though, hopefully there isn't much stderr in builds that succeed.
There was a problem hiding this comment.
Should we collect stderr and then log it manually in that case?
There was a problem hiding this comment.
Taking a step back, I know this PR didn't change this to sync, and I first I was thinking this just ran on startup, but I think this build does actually run when there is a deploy component operation on any of the standard worker threads (https://github.com/HarperDB/harperdb/blob/main/components/operations.js#L427). Consequently, I think this should really all be executing asynchronously. As a general rule, once a thread is serving (HTTP or otherwise) requests, we want to avoid any execution that pauses the event loop for more than probably ~10ms. And I think this build can definitely take much longer than that. So I think we should probably use the async exec, which I believe has its own system for handling stdio.
There was a problem hiding this comment.
okay I'll see what I can do
I noticed a bug in this when you pass a command like
echo foo && next buildit would fail because of the command parsing/handling. It is simply easier to use execSync instead.