makes exec_auth spawn the command asynchronously #2046
Conversation
k8s-ci-robot
added
the
do-not-merge/invalid-commit-message
k8s-ci-robot
added
the
needs-rebase
k8s-ci-robot
added
cncf-cla: yes
|
Many thanks for your review. I've made the changes and pushed a new commit |
src/exec_auth.ts
Outdated
| subprocess.stdout.setEncoding('utf8'); | ||
| subprocess.stderr.setEncoding('utf8'); | ||
|
|
||
| subprocess.stdout.on('data', (data: Buffer | string) => { |
There was a problem hiding this comment.
The Buffer part of the type can be dropped now (not sure if TypeScript will complain about it though).
src/exec_auth.ts
Outdated
| subprocess.stdout.on('data', (data: Buffer | string) => { | ||
| stdoutData += data.toString('utf8'); |
There was a problem hiding this comment.
The toString() call should be dropped now.
src/exec_auth.ts
Outdated
| }); | ||
|
|
||
| subprocess.stderr.on('data', (data) => { | ||
| stderrData += data.toString('utf8'); |
There was a problem hiding this comment.
Same comment here about dropping the toString().
src/exec_auth.ts
Outdated
|
|
||
| subprocess.on('error', (error) => { | ||
| savedError = error; | ||
| throw error; |
There was a problem hiding this comment.
I'm not sure that it still makes sense to throw here.
There was a problem hiding this comment.
my mistake, I forgot to remove it
|
LGTM, thanks! |
|
/lgtm Thanks for the PR! |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, feloy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
This PR makes exec_auth calls
spawninstead ofspawnSync.This is cery useful for a GUI to continue working during the plugin is executed.
Fixes #2044