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

Identify conda and windows store environments from given interpreter path #13589

Merged
merged 8 commits into from
Sep 1, 2020

Conversation

karthiknadig
Copy link
Member

@karthiknadig karthiknadig commented Aug 24, 2020

This API is not consumed any where. This is adding all the low-level APIs we need for environment identification. In this PR, there is code to identify conda and windows store environments.

@karthiknadig karthiknadig added the no-changelog No news entry required label Aug 24, 2020
@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2020

Codecov Report

Merging #13589 into master will decrease coverage by 0.06%.
The diff coverage is 75.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13589      +/-   ##
==========================================
- Coverage   59.86%   59.80%   -0.07%     
==========================================
  Files         670      675       +5     
  Lines       37295    37798     +503     
  Branches     5335     5445     +110     
==========================================
+ Hits        22326    22604     +278     
- Misses      13815    14019     +204     
- Partials     1154     1175      +21     
Impacted Files Coverage Δ
...t/pythonEnvironments/discovery/locators/helpers.ts 70.00% <0.00%> (-0.59%) ⬇️
src/client/common/utils/platform.ts 56.00% <37.50%> (-20.48%) ⬇️
...pythonEnvironments/common/environmentIdentifier.ts 87.50% <87.50%> (ø)
src/client/pythonEnvironments/info/index.ts 41.53% <0.00%> (-27.43%) ⬇️
src/client/datascience/crossProcessLock.ts 79.41% <0.00%> (-11.77%) ⬇️
src/client/datascience/jupyter/kernels/helpers.ts 35.51% <0.00%> (-10.27%) ⬇️
...rc/client/pythonEnvironments/info/pythonVersion.ts 87.87% <0.00%> (-8.68%) ⬇️
...nt/datascience/notebookStorage/vscNotebookModel.ts 61.11% <0.00%> (-4.80%) ⬇️
src/client/datascience/jupyter/jupyterSession.ts 72.34% <0.00%> (-4.77%) ⬇️
... and 62 more

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 3fd9e8a...0341c10. Read the comment docs.

@karthiknadig karthiknadig changed the title Partial: Identify conda and windows store environments from given interpreter path Identify conda and windows store environments from given interpreter path Aug 25, 2020
@karthiknadig karthiknadig marked this pull request as ready for review August 25, 2020 19:21
@int19h
Copy link

int19h commented Aug 25, 2020

Should this logic really be in a single low-level component? It seems strange that there's a bunch of code that's "common", but which contains conda-specific details in practice - shouldn't that all be in the conda locator instead?

@karthiknadig
Copy link
Member Author

Since we don't have the templates for the (new) locators ready. I am putting them here for now. My plan is to move the isConda, isWindowsStore, etc, into respective locators. Even there i think these would be exposed as flat functions. Since they don't really need any state.

As for the identifyEnvironment function itself, is supposed to be a generic identifier, which should handle cases where user just provides a path. When we have locator manager, I will use that to run through and get the environment type. That way I can get rid of calling specific locator checks. and ensure that we prioritize the type appropriately.

@sonarcloud
Copy link

sonarcloud bot commented Sep 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
0.0% 0.0% Duplication

@karthiknadig karthiknadig merged commit 869ec10 into microsoft:master Sep 1, 2020
@karrtikr karrtikr mentioned this pull request Sep 4, 2020
10 tasks
@karthiknadig karthiknadig deleted the identify branch October 23, 2020 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog No news entry required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants