Fixes related to loading of env variables #10843
Conversation
|
|
||
| // If user asks us to, set PYTHONNOUSERSITE | ||
| // For more details see here https://github.com/microsoft/vscode-jupyter/issues/8553#issuecomment-997144591 | ||
| // https://docs.python.org/3/library/site.html#site.ENABLE_USER_SITE |
There was a problem hiding this comment.
This duplicate code can be replaced with KernelEnvironmentVariablesService.getEnvironmentVariables
There was a problem hiding this comment.
Now both raw kernels and jupyter kernels will use the same set of code to generate env variables for staring of kernels.
| traceInfo( | ||
| `No custom variables for Kernel as interpreter path is not defined for kernel ${kernelSpec.display_name}` | ||
| ); | ||
| return kernelEnv; |
There was a problem hiding this comment.
We will always return the env variables in this method, never undefined
i.e. merge process.env with kernelspec.env and the like.
| // Also applies to `!java` where java could be an executable in the conda bin directory. | ||
| if (interpreter) { | ||
| const env = kernelEnv || process.env; | ||
| this.envVarsService.prependPath(env, path.dirname(interpreter.uri.fsPath)); |
There was a problem hiding this comment.
This isn't necessary as we already do this further below (exact same code)
There was a problem hiding this comment.
But below doesn't return early? It doesn't look the same to me?
There was a problem hiding this comment.
Thats correct, however if you check the code below its the same, as kernelEnv and customEnvVars will be empty, hence most of the code below will be noops, but at least this way its consistent, else we have a lot of branching if conditions, which just complicates it.
| } | ||
| otherEnvPathKey = Object.keys(kernelEnv).find((k) => k.toLowerCase() == 'path'); | ||
| if (otherEnvPathKey && kernelEnv[otherEnvPathKey]) { | ||
| this.envVarsService.appendPath(mergedVars, kernelEnv[otherEnvPathKey]!); | ||
| } | ||
| if (customEditVars.PYTHONPATH) { |
There was a problem hiding this comment.
renamed to make is clear that these are custom Environment variables. I am guessing the Edit was a typo from my end.
Codecov Report
@@ Coverage Diff @@
## main #10843 +/- ##
======================================
Coverage 63% 63%
======================================
Files 482 482
Lines 33652 33600 -52
Branches 5488 5471 -17
======================================
- Hits 21278 21276 -2
+ Misses 10320 10294 -26
+ Partials 2054 2030 -24
|
| @@ -276,75 +274,35 @@ export class JupyterKernelService implements IJupyterKernelService { | |||
| ); | |||
| specModel.argv[0] = interpreter.uri.fsPath; | |||
| } | |||
| // Get the activated environment variables (as a work around for `conda run` and similar). | |||
| // This ensures the code runs within the context of an activated environment. | |||
| const interpreterEnv = await this.activationHelper | |||
There was a problem hiding this comment.
Removed duplicate code in favor of kernelEnvVars.getEnvironmentVariables
Now both raw and jupyter kernels (both) use the exact same code to generate env variables for kernels.
|
I still cannot get this to work. How do I set environment variables from a |
Part of #10359