refactor: Adapt to async version of spawn#648
refactor: Adapt to async version of spawn#648pgrzesik merged 1 commit intomasterserverless/serverless-python-requirements:masterfrom migrate-spawn-sync-to-async-versionserverless/serverless-python-requirements:migrate-spawn-sync-to-async-versionCopy head branch name to clipboard
async version of spawn#648Conversation
| try { | ||
| const pipTestRes = await spawn(pythonBin, ['-m', 'pip', 'help', 'install']); | ||
| return ( | ||
| pipTestRes.stdoutBuffer && |
There was a problem hiding this comment.
stdoutBuffer will be provided unconditionally, so no need to confirm on it
There was a problem hiding this comment.
Is that the same case with stderrBuffer?
There was a problem hiding this comment.
Yes, but in case of error handling, there could be an edge case, where the command fails because it cannot be invoked for some reason and then the error won't hold any of those properties
| const pipTestRes = await spawn(pythonBin, ['-m', 'pip', 'help', 'install']); | ||
| return ( | ||
| pipTestRes.stdoutBuffer && | ||
| pipTestRes.stdoutBuffer.toString().indexOf('--system') >= 0 |
There was a problem hiding this comment.
More natural would be includes('--system')
|
Hello @obataku - I believe it's not supported at all on Buffer, are you observing some issues or such usage? |
|
@pgrzesik indeed, using ... which points to the following change introduced by this PR: https://github.com/serverless/serverless-python-requirements/blame/e3d9ebcdd3ec72cc2e3301d33e10dd10ef6a594d/lib/docker.js#L200 - return ps.stdout.trim();
+ return ps.stdoutBuffer.trim(); |
|
Thanks for reporting, I see the issue now where the |
Related to: #646 - sync use of spawn prevented interactive output to be updated