Fix for running in electron environment #59
Conversation
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@mkramb we need you to sign the CNCF CLA. Thanks! |
|
@mkramb does this actually work? This is changing the call from being synchronous to being asynchronous and I worry that there is a race between token update and web request generation... (separately I'm having similar issues for the Azure provider, so we do need to deal with async clients somehow here...) |
|
I have signed CLA. It works (tested in electron environment) and also existing tests are passing, also the attached issue - if you look at the last comment from the ShellJS maintainer, it looks like best solution ATM. |
|
This needs a rebase. I still am not confident that this will work correctly given that it is switching the call from sync to async, it feels racy to me... (and I'm not positive the tests cover that particular part of the code, sorry!) Any chance that you can conditionalize this so that it only works this way when running in electron? Thanks! |
|
Would a safe way forward on this be to create Most places within the code that call It would be a much larger piece of work though (and not lacking in risk either). |
|
This doesn't actually work, it just swallows the error because the function returns immediately, and doesn't have an error event handler. I would suggest switching from |
I'm using
kubernetes-clientinelectronenvironment (in main/node renderer) and the underlying libraryshelljshas issues when running as an electron package application. More information here:Based on testing the cleanest solution is setting exec to async mode, which doesn't have this problem. I'm using this in https://github.com/mkramb/jx-tray which is a start of https://jenkins-x.io desktop client.
No changes in functionality & existing tests are passing.