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

Implemented new interpreter storage supporting multiroot workspaces #10325

Merged
merged 73 commits into from
Apr 1, 2020

Conversation

karrtikr
Copy link

@karrtikr karrtikr commented Feb 26, 2020

Sorry for such a big PR, do let me know if I need to break it!

For #10375

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).
  • The wiki is updated with any design decisions/details.

@codecov-io
Copy link

codecov-io commented Feb 26, 2020

Codecov Report

Merging #10325 into pythonPath will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           pythonPath   #10325   +/-   ##
===========================================
  Coverage       60.95%   60.95%           
===========================================
  Files             585      585           
  Lines           31817    31817           
  Branches         4524     4524           
===========================================
  Hits            19393    19393           
  Misses          11444    11444           
  Partials          980      980           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc8774d...fc8774d. Read the comment docs.

@karrtikr karrtikr changed the title Implemented new interpreter storage supporting multiroot workspaces [WIP] Implemented new interpreter storage supporting multiroot workspaces Feb 27, 2020
@karrtikr karrtikr force-pushed the deprecatePythonPath branch 2 times, most recently from a4bca7e to 2cda0f3 Compare February 29, 2020 12:53
@karrtikr karrtikr marked this pull request as ready for review February 29, 2020 14:06
@karrtikr karrtikr changed the base branch from master to pythonPath February 29, 2020 14:10
@karrtikr karrtikr changed the base branch from pythonPath to master February 29, 2020 14:15
@karrtikr karrtikr changed the base branch from master to pythonPath February 29, 2020 14:19
Copy link

@kimadeline kimadeline left a comment

Choose a reason for hiding this comment

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

I'm only halfway done with my review, but I had to restart my computer 😅

src/client/common/application/workspace.ts Outdated Show resolved Hide resolved
src/client/common/interpreterPathService.ts Outdated Show resolved Hide resolved
src/client/common/interpreterPathService.ts Outdated Show resolved Hide resolved
src/client/common/interpreterPathService.ts Show resolved Hide resolved
src/client/common/types.ts Outdated Show resolved Hide resolved
src/client/extension.ts Outdated Show resolved Hide resolved
src/client/interpreter/autoSelection/rules/workspaceEnv.ts Outdated Show resolved Hide resolved
src/client/interpreter/display/index.ts Show resolved Hide resolved
@karrtikr karrtikr force-pushed the deprecatePythonPath branch 4 times, most recently from a93ec74 to e43d227 Compare March 5, 2020 16:07
@karrtikr karrtikr force-pushed the deprecatePythonPath branch 5 times, most recently from 4bf7390 to 9044207 Compare March 24, 2020 20:40
@karrtikr karrtikr changed the title [WIP] Implemented new interpreter storage supporting multiroot workspaces Implemented new interpreter storage supporting multiroot workspaces Mar 30, 2020
src/client/common/configSettings.ts Show resolved Hide resolved
src/client/common/configSettings.ts Outdated Show resolved Hide resolved
src/client/common/interpreterPathService.ts Outdated Show resolved Hide resolved
src/client/common/interpreterPathService.ts Outdated Show resolved Hide resolved
src/client/common/configSettings.ts Outdated Show resolved Hide resolved
src/client/common/interpreterPathService.ts Outdated Show resolved Hide resolved
src/client/common/utils/cacheUtils.ts Outdated Show resolved Hide resolved
src/client/interpreter/display/index.ts Outdated Show resolved Hide resolved
src/test/common/configuration/service.unit.test.ts Outdated Show resolved Hide resolved
src/test/common/configuration/service.unit.test.ts Outdated Show resolved Hide resolved
src/test/common/configuration/service.unit.test.ts Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Apr 1, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@karrtikr karrtikr removed the request for review from DonJayamanne April 1, 2020 21:15
@karrtikr karrtikr merged commit 776efa5 into microsoft:pythonPath Apr 1, 2020
@karrtikr karrtikr deleted the deprecatePythonPath branch April 1, 2020 21:15
@karrtikr karrtikr restored the deprecatePythonPath branch April 1, 2020 21:20
@karrtikr karrtikr deleted the deprecatePythonPath branch April 1, 2020 21:20
karrtikr pushed a commit that referenced this pull request Apr 7, 2020
…10325)

* Added python.defaultInterpreterPath setting

* Created persistent storage infrastructure

* Added vscode.workspace.workspaceFile API

* Implemented inspect interpreter path

* Implement config service updater for pythonPath

* Correct build errors

* Change extension to use new infrastructure

* Store service container in a global variable

* Update settings in the new way src/test/common

* Correct cyclic dependency between IExperimentsManager and IConfigurationService

* Detect change in new interpreter storage and act accordingly

* Added reset python interpreters command

* Cache the auto selected interpreter

* Log python interpreter in the output channel

* Fix autoselection and speed up the interpreter change process

* Reset experiments

* Reset python interpreters for workspace

* Remove reset interpreter command from this PR

* Added news entry

* Update getWorkspaceFolderIdentifier

* Code reviews

* Dispose handler

* More code reviews

* Resolve merge conflicts

* Fix issues

* Some more fixes

* More fixes

* Do not assume ACTIVATED_SERVICE_CONTAINER is defined

* Fix running tests

* Fix cacheUtils.ts and test/common.ts

* Correct cacheResourceSpecificInterpreterData decorator

* Added tests for decorator-like implementation in enviroment provider

* Code reviews

* Fix interpreter service unit tests

* Fix interpreter display unit tests

* Fix mac interpreter unit tests

* Fix interpreter selector unit tests

* Fix configSettings pythonPath unit tests

* Fix configSettings unit tests

* Rebase with master

* Fix installer.test.ts tests

* Fix moduleInstaller.test.ts tests

* Fix pythonPathUpdater.test.ts tests

* Fix pythonProc.simple.multiroot.test.ts tests

* Fix data science functional tests

* Fix smoke tests

* Use user friendly path in the output channel

* Use symbols directly

* Fix bug with configSettings.ts

* Fix venv tests

* Run all tests with the new interpreter storage"

* Restore YAML pipeline

* Fix absolute path resolver

* Run all tests with old interpreter storage

* Reduce run time of venv tests

* Run only mac multiroot

* Remove experiment from experiments.json

* Added tests for Interpreter path service

* saa

* Ensure we use `CI_PYTHON_PATH` when running tests with the new interpreter storage

* Reset global interpreter path as well

* Added tests for mac interpreter diagnostic

* Added tests for src\client\interpreter\autoSelection\rules\settings.ts

* Added tests for src\client\interpreter\configuration\pythonPathUpdaterServiceFactory.ts

* Added tests for src\client\interpreter\autoSelection\rules\workspaceEnv.ts

* Added tests for src\client\interpreter\interpreterService.ts

* Added tests for src\client\common\configuration\service.ts

* Added tests for src\client\startupTelemetry.ts

* Correct tests

* Module installer tests

* Code reviews

* More code reviews

* Fix lint error
karrtikr pushed a commit that referenced this pull request Apr 7, 2020
…10325)

* Added python.defaultInterpreterPath setting

* Created persistent storage infrastructure

* Added vscode.workspace.workspaceFile API

* Implemented inspect interpreter path

* Implement config service updater for pythonPath

* Correct build errors

* Change extension to use new infrastructure

* Store service container in a global variable

* Update settings in the new way src/test/common

* Correct cyclic dependency between IExperimentsManager and IConfigurationService

* Detect change in new interpreter storage and act accordingly

* Added reset python interpreters command

* Cache the auto selected interpreter

* Log python interpreter in the output channel

* Fix autoselection and speed up the interpreter change process

* Reset experiments

* Reset python interpreters for workspace

* Remove reset interpreter command from this PR

* Added news entry

* Update getWorkspaceFolderIdentifier

* Code reviews

* Dispose handler

* More code reviews

* Resolve merge conflicts

* Fix issues

* Some more fixes

* More fixes

* Do not assume ACTIVATED_SERVICE_CONTAINER is defined

* Fix running tests

* Fix cacheUtils.ts and test/common.ts

* Correct cacheResourceSpecificInterpreterData decorator

* Added tests for decorator-like implementation in enviroment provider

* Code reviews

* Fix interpreter service unit tests

* Fix interpreter display unit tests

* Fix mac interpreter unit tests

* Fix interpreter selector unit tests

* Fix configSettings pythonPath unit tests

* Fix configSettings unit tests

* Rebase with master

* Fix installer.test.ts tests

* Fix moduleInstaller.test.ts tests

* Fix pythonPathUpdater.test.ts tests

* Fix pythonProc.simple.multiroot.test.ts tests

* Fix data science functional tests

* Fix smoke tests

* Use user friendly path in the output channel

* Use symbols directly

* Fix bug with configSettings.ts

* Fix venv tests

* Run all tests with the new interpreter storage"

* Restore YAML pipeline

* Fix absolute path resolver

* Run all tests with old interpreter storage

* Reduce run time of venv tests

* Run only mac multiroot

* Remove experiment from experiments.json

* Added tests for Interpreter path service

* saa

* Ensure we use `CI_PYTHON_PATH` when running tests with the new interpreter storage

* Reset global interpreter path as well

* Added tests for mac interpreter diagnostic

* Added tests for src\client\interpreter\autoSelection\rules\settings.ts

* Added tests for src\client\interpreter\configuration\pythonPathUpdaterServiceFactory.ts

* Added tests for src\client\interpreter\autoSelection\rules\workspaceEnv.ts

* Added tests for src\client\interpreter\interpreterService.ts

* Added tests for src\client\common\configuration\service.ts

* Added tests for src\client\startupTelemetry.ts

* Correct tests

* Module installer tests

* Code reviews

* More code reviews

* Fix lint error
karrtikr pushed a commit that referenced this pull request Apr 7, 2020
…10325)

* Added python.defaultInterpreterPath setting

* Created persistent storage infrastructure

* Added vscode.workspace.workspaceFile API

* Implemented inspect interpreter path

* Implement config service updater for pythonPath

* Correct build errors

* Change extension to use new infrastructure

* Store service container in a global variable

* Update settings in the new way src/test/common

* Correct cyclic dependency between IExperimentsManager and IConfigurationService

* Detect change in new interpreter storage and act accordingly

* Added reset python interpreters command

* Cache the auto selected interpreter

* Log python interpreter in the output channel

* Fix autoselection and speed up the interpreter change process

* Reset experiments

* Reset python interpreters for workspace

* Remove reset interpreter command from this PR

* Added news entry

* Update getWorkspaceFolderIdentifier

* Code reviews

* Dispose handler

* More code reviews

* Resolve merge conflicts

* Fix issues

* Some more fixes

* More fixes

* Do not assume ACTIVATED_SERVICE_CONTAINER is defined

* Fix running tests

* Fix cacheUtils.ts and test/common.ts

* Correct cacheResourceSpecificInterpreterData decorator

* Added tests for decorator-like implementation in enviroment provider

* Code reviews

* Fix interpreter service unit tests

* Fix interpreter display unit tests

* Fix mac interpreter unit tests

* Fix interpreter selector unit tests

* Fix configSettings pythonPath unit tests

* Fix configSettings unit tests

* Rebase with master

* Fix installer.test.ts tests

* Fix moduleInstaller.test.ts tests

* Fix pythonPathUpdater.test.ts tests

* Fix pythonProc.simple.multiroot.test.ts tests

* Fix data science functional tests

* Fix smoke tests

* Use user friendly path in the output channel

* Use symbols directly

* Fix bug with configSettings.ts

* Fix venv tests

* Run all tests with the new interpreter storage"

* Restore YAML pipeline

* Fix absolute path resolver

* Run all tests with old interpreter storage

* Reduce run time of venv tests

* Run only mac multiroot

* Remove experiment from experiments.json

* Added tests for Interpreter path service

* saa

* Ensure we use `CI_PYTHON_PATH` when running tests with the new interpreter storage

* Reset global interpreter path as well

* Added tests for mac interpreter diagnostic

* Added tests for src\client\interpreter\autoSelection\rules\settings.ts

* Added tests for src\client\interpreter\configuration\pythonPathUpdaterServiceFactory.ts

* Added tests for src\client\interpreter\autoSelection\rules\workspaceEnv.ts

* Added tests for src\client\interpreter\interpreterService.ts

* Added tests for src\client\common\configuration\service.ts

* Added tests for src\client\startupTelemetry.ts

* Correct tests

* Module installer tests

* Code reviews

* More code reviews

* Fix lint error
karrtikr pushed a commit that referenced this pull request Apr 8, 2020
…10325)

* Added python.defaultInterpreterPath setting

* Created persistent storage infrastructure

* Added vscode.workspace.workspaceFile API

* Implemented inspect interpreter path

* Implement config service updater for pythonPath

* Correct build errors

* Change extension to use new infrastructure

* Store service container in a global variable

* Update settings in the new way src/test/common

* Correct cyclic dependency between IExperimentsManager and IConfigurationService

* Detect change in new interpreter storage and act accordingly

* Added reset python interpreters command

* Cache the auto selected interpreter

* Log python interpreter in the output channel

* Fix autoselection and speed up the interpreter change process

* Reset experiments

* Reset python interpreters for workspace

* Remove reset interpreter command from this PR

* Added news entry

* Update getWorkspaceFolderIdentifier

* Code reviews

* Dispose handler

* More code reviews

* Resolve merge conflicts

* Fix issues

* Some more fixes

* More fixes

* Do not assume ACTIVATED_SERVICE_CONTAINER is defined

* Fix running tests

* Fix cacheUtils.ts and test/common.ts

* Correct cacheResourceSpecificInterpreterData decorator

* Added tests for decorator-like implementation in enviroment provider

* Code reviews

* Fix interpreter service unit tests

* Fix interpreter display unit tests

* Fix mac interpreter unit tests

* Fix interpreter selector unit tests

* Fix configSettings pythonPath unit tests

* Fix configSettings unit tests

* Rebase with master

* Fix installer.test.ts tests

* Fix moduleInstaller.test.ts tests

* Fix pythonPathUpdater.test.ts tests

* Fix pythonProc.simple.multiroot.test.ts tests

* Fix data science functional tests

* Fix smoke tests

* Use user friendly path in the output channel

* Use symbols directly

* Fix bug with configSettings.ts

* Fix venv tests

* Run all tests with the new interpreter storage"

* Restore YAML pipeline

* Fix absolute path resolver

* Run all tests with old interpreter storage

* Reduce run time of venv tests

* Run only mac multiroot

* Remove experiment from experiments.json

* Added tests for Interpreter path service

* saa

* Ensure we use `CI_PYTHON_PATH` when running tests with the new interpreter storage

* Reset global interpreter path as well

* Added tests for mac interpreter diagnostic

* Added tests for src\client\interpreter\autoSelection\rules\settings.ts

* Added tests for src\client\interpreter\configuration\pythonPathUpdaterServiceFactory.ts

* Added tests for src\client\interpreter\autoSelection\rules\workspaceEnv.ts

* Added tests for src\client\interpreter\interpreterService.ts

* Added tests for src\client\common\configuration\service.ts

* Added tests for src\client\startupTelemetry.ts

* Correct tests

* Module installer tests

* Code reviews

* More code reviews

* Fix lint error
karrtikr pushed a commit that referenced this pull request Apr 8, 2020
…10325)

* Added python.defaultInterpreterPath setting

* Created persistent storage infrastructure

* Added vscode.workspace.workspaceFile API

* Implemented inspect interpreter path

* Implement config service updater for pythonPath

* Correct build errors

* Change extension to use new infrastructure

* Store service container in a global variable

* Update settings in the new way src/test/common

* Correct cyclic dependency between IExperimentsManager and IConfigurationService

* Detect change in new interpreter storage and act accordingly

* Added reset python interpreters command

* Cache the auto selected interpreter

* Log python interpreter in the output channel

* Fix autoselection and speed up the interpreter change process

* Reset experiments

* Reset python interpreters for workspace

* Remove reset interpreter command from this PR

* Added news entry

* Update getWorkspaceFolderIdentifier

* Code reviews

* Dispose handler

* More code reviews

* Resolve merge conflicts

* Fix issues

* Some more fixes

* More fixes

* Do not assume ACTIVATED_SERVICE_CONTAINER is defined

* Fix running tests

* Fix cacheUtils.ts and test/common.ts

* Correct cacheResourceSpecificInterpreterData decorator

* Added tests for decorator-like implementation in enviroment provider

* Code reviews

* Fix interpreter service unit tests

* Fix interpreter display unit tests

* Fix mac interpreter unit tests

* Fix interpreter selector unit tests

* Fix configSettings pythonPath unit tests

* Fix configSettings unit tests

* Rebase with master

* Fix installer.test.ts tests

* Fix moduleInstaller.test.ts tests

* Fix pythonPathUpdater.test.ts tests

* Fix pythonProc.simple.multiroot.test.ts tests

* Fix data science functional tests

* Fix smoke tests

* Use user friendly path in the output channel

* Use symbols directly

* Fix bug with configSettings.ts

* Fix venv tests

* Run all tests with the new interpreter storage"

* Restore YAML pipeline

* Fix absolute path resolver

* Run all tests with old interpreter storage

* Reduce run time of venv tests

* Run only mac multiroot

* Remove experiment from experiments.json

* Added tests for Interpreter path service

* saa

* Ensure we use `CI_PYTHON_PATH` when running tests with the new interpreter storage

* Reset global interpreter path as well

* Added tests for mac interpreter diagnostic

* Added tests for src\client\interpreter\autoSelection\rules\settings.ts

* Added tests for src\client\interpreter\configuration\pythonPathUpdaterServiceFactory.ts

* Added tests for src\client\interpreter\autoSelection\rules\workspaceEnv.ts

* Added tests for src\client\interpreter\interpreterService.ts

* Added tests for src\client\common\configuration\service.ts

* Added tests for src\client\startupTelemetry.ts

* Correct tests

* Module installer tests

* Code reviews

* More code reviews

* Fix lint error
karrtikr pushed a commit that referenced this pull request Apr 8, 2020
* Modified `Select interpreter` command & add a reset interpreter command (#10373)

* Modified `Select interpreter` command to support setting interpreter at workspace level

* Added a command to reset value of Python interpreter

* Code reviews

* Update src/client/interpreter/configuration/interpreterSelector.ts

Co-Authored-By: Karthik Nadig <kanadig@microsoft.com>

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>

* Implemented new interpreter storage supporting multiroot workspaces (#10325)

* Added python.defaultInterpreterPath setting

* Created persistent storage infrastructure

* Added vscode.workspace.workspaceFile API

* Implemented inspect interpreter path

* Implement config service updater for pythonPath

* Correct build errors

* Change extension to use new infrastructure

* Store service container in a global variable

* Update settings in the new way src/test/common

* Correct cyclic dependency between IExperimentsManager and IConfigurationService

* Detect change in new interpreter storage and act accordingly

* Added reset python interpreters command

* Cache the auto selected interpreter

* Log python interpreter in the output channel

* Fix autoselection and speed up the interpreter change process

* Reset experiments

* Reset python interpreters for workspace

* Remove reset interpreter command from this PR

* Added news entry

* Update getWorkspaceFolderIdentifier

* Code reviews

* Dispose handler

* More code reviews

* Resolve merge conflicts

* Fix issues

* Some more fixes

* More fixes

* Do not assume ACTIVATED_SERVICE_CONTAINER is defined

* Fix running tests

* Fix cacheUtils.ts and test/common.ts

* Correct cacheResourceSpecificInterpreterData decorator

* Added tests for decorator-like implementation in enviroment provider

* Code reviews

* Fix interpreter service unit tests

* Fix interpreter display unit tests

* Fix mac interpreter unit tests

* Fix interpreter selector unit tests

* Fix configSettings pythonPath unit tests

* Fix configSettings unit tests

* Rebase with master

* Fix installer.test.ts tests

* Fix moduleInstaller.test.ts tests

* Fix pythonPathUpdater.test.ts tests

* Fix pythonProc.simple.multiroot.test.ts tests

* Fix data science functional tests

* Fix smoke tests

* Use user friendly path in the output channel

* Use symbols directly

* Fix bug with configSettings.ts

* Fix venv tests

* Run all tests with the new interpreter storage"

* Restore YAML pipeline

* Fix absolute path resolver

* Run all tests with old interpreter storage

* Reduce run time of venv tests

* Run only mac multiroot

* Remove experiment from experiments.json

* Added tests for Interpreter path service

* saa

* Ensure we use `CI_PYTHON_PATH` when running tests with the new interpreter storage

* Reset global interpreter path as well

* Added tests for mac interpreter diagnostic

* Added tests for src\client\interpreter\autoSelection\rules\settings.ts

* Added tests for src\client\interpreter\configuration\pythonPathUpdaterServiceFactory.ts

* Added tests for src\client\interpreter\autoSelection\rules\workspaceEnv.ts

* Added tests for src\client\interpreter\interpreterService.ts

* Added tests for src\client\common\configuration\service.ts

* Added tests for src\client\startupTelemetry.ts

* Correct tests

* Module installer tests

* Code reviews

* More code reviews

* Fix lint error

* Prompt when an "unsafe" workspace python environment is to be autoselected  (#10430)

* Implement prompt when an unsafe workspace python environment is to be autoselected

* Code reviews

* Refactor pythonPath stuff into a private method

* Use variable instead of hardcoding 'python' string

* Added tests for src\client\interpreter\autoSelection\interpreterSecurity\interpreterSecurityCommands.ts

* Don't register command in the constructor

* Refactored code

* Updated tests

* Code reviews

* Moved bannerLabelYes/No to Common() namespace

* Update content for news entry

* Corrected activation manager tests

* Fix single workspace CI tests

* Fix multiroot tests + Code reviews

* Rename Interpreter Security clear command

* Fix tests

* Added intellisense for pythonpath experiment

* Fix multiroot tests

* Rename `Reset interpreter` command to `Clear workspace interpreter setting

* Fix bug with interpreter security storage

* Correct prettier errors

* Edit news entries appropriately

* Git rebase

* Code reviews

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants