Connect to raw kernel with command line args instead of connection file #11894
Conversation
| @@ -271,7 +271,7 @@ | |||
| ], | |||
| "env": { | |||
| // Remove `X` prefix to test with real browser to host DS ui (for DS functional tests). | |||
| "xVSC_PYTHON_DS_UI_BROWSER": "1", | |||
| "XVSC_PYTHON_DS_UI_BROWSER": "1", | |||
There was a problem hiding this comment.
That little x was bugging me 😆
| // Update the args in the kernelspec to include the conenction file. | ||
| // Instead of having to use a connection file update our local copy of the kernelspec to launch | ||
| // directly with command line arguments | ||
| private updateConnectionArgs() { |
There was a problem hiding this comment.
I thought briefly about where to handle this and I believe this is the right spot.
- We already make a deep copy of the kernelspec in KernelProcess so that we can mess with it, but not change the original spec
- Same location we were previously creating and inserting the connection file info.
There was a problem hiding this comment.
I tested on windows as well. Not fully sure if this fixes the nightly test runs (as those issues don't seem to repro doing real python tests locally) so I'll queue up a run for that. I would expect this to fix the nightly test issues but even if there was another issue there this new method seems like a solid improvement, no messing around with creating and removing files.
IanMatthewHuff
requested review from
DonJayamanne,
rchiodo,
DavidKutu and
greazer
|
Kudos, SonarCloud Quality Gate passed!
|
|
Hey did you try a nightly run? Can't seem to find it. Was curious if it worked or not. |
|
@rchiodo Just queued it up. Just didn't do that until after lunch. I'm going to submit regardless as I think it's a good change anyways. Fingers crossed that it fixes the windows flakes as well. |
For #11883
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).