Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dont set PYTHONNOUSERSITE unless explicitly asked #10124

Merged
merged 1 commit into from
May 25, 2022

Conversation

rchiodo
Copy link
Contributor

@rchiodo rchiodo commented May 25, 2022

Fixes #9995

Tested this with

  • global kernel
  • venv kernel
  • conda kernel
  • same with using jupyter instead of raw

@rchiodo rchiodo requested a review from a team as a code owner May 25, 2022 00:08
@@ -39,7 +39,7 @@ export class JupyterInterpreterStateStore {
}
const memento = this.memento.get<string | undefined>(key, undefined);
if (memento) {
return Uri.file(memento);
return Uri.parse(memento);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causing problems with setting the jupyter interpreter. Caused by the string to URI change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really sucks because if we fail to get this, we clear it. So a lot of people might have had their interpreter cleared we set a long time ago.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S**t happens...

"jupyter.excludeUserSitePackages": {
"type": "boolean",
"default": false,
"description": "Add PYTHONNOUSERSITE to kernels before starting. This prevents global/user site-packages from being used in the PYTHONPATH of the kernel.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this is an option in case we break anybody. We can have them set this to try the old way. Telemetry should tell us how many people have this set.

@codecov-commenter
Copy link

Codecov Report

Merging #10124 (6ce07d5) into main (c5882ac) will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           main   #10124   +/-   ##
=====================================
  Coverage    55%      55%           
=====================================
  Files       203      203           
  Lines      9243     9244    +1     
  Branches   1492     1492           
=====================================
+ Hits       5092     5093    +1     
  Misses     3747     3747           
  Partials    404      404           
Impacted Files Coverage Δ
src/platform/common/types.ts 100% <ø> (ø)
src/platform/common/configSettings.ts 80% <100%> (+<1%) ⬆️

@rchiodo rchiodo merged commit bc74fc7 into main May 25, 2022
@rchiodo rchiodo deleted the rchiodo/remove_pythonnousersite branch May 25, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PYTHONNOUSERSITE setting isn't being honored.
3 participants