Skip to content

Always use environment path when running conda environment commands - #24807

Merged
Karthik Nadig (karthiknadig) merged 2 commits into
microsoft:mainfrom
jpcorreia99:jcorreia/useCondaPAth
Feb 14, 2025
Merged

Always use environment path when running conda environment commands#24807
Karthik Nadig (karthiknadig) merged 2 commits into
microsoft:mainfrom
jpcorreia99:jcorreia/useCondaPAth

Conversation

@jpcorreia99

Copy link
Copy Markdown

Attempt at fixing #24585

There are many edge scenarious where refering to the name of the environment rather than the path can cause breaks in the extension. Some examples

1 -If we have two anonymous environments with the same name in different folders

/path1/my-env
/path2/my-env (where my active vscode python interpreter is)
by using conda -n my-env it'll always use the first env.

2 - Some times people avoid actually activating their conda envs when using conda-pack https://github.com/conda/conda-pack

This is because the activation scripts are known to be flaky and not very reliable

3 - The environment may have been created by a conda-compliant replacement

Therefore conda itself is not aware of it by name but can work with it properly using the path. This is the case of hawk or frankly anyone building their own conda package manager on top of rattler.

Some of these points are also hinted at #24627 (comment) , and supported by a conda maintainer in #24585 (comment)

This PR has a minimal attempt at changing that by always forcing -p usage

@jpcorreia99

Copy link
Copy Markdown
Author

Eleanor Boyd (@eleanorjboyd)
Karthik Nadig (@karthiknadig)
Anthony Kim (@anthonykim1)

While I work on getting the CLA approved, would really appreciate if someone could give this a look!

@vs-code-engineering vs-code-engineering Bot added this to the February 2025 milestone Feb 11, 2025
@karthiknadig

Copy link
Copy Markdown
Member

João Correia (@jpcorreia99) This is a great idea to always use the prefix for the run scenario. We can give this a try and see if it work in all cases. From what I can tell it should.

auto-merge was automatically disabled February 11, 2025 15:36

Head branch was pushed to by a user without write access

@jpcorreia99

João Correia (jpcorreia99) commented Feb 12, 2025

Copy link
Copy Markdown
Author

hey Karthik Nadig (@karthiknadig) could you please reapprove the PR 🙏 Had to add an extra commit due to a unit test I had missed. CLA has been signed! (and you may want to remove the other reviewer if possible)

Also, is there documentation regarding the release cycle of the extension? Would be interested in knowing when I can expect this change to be available.

Thank you for the review!

@jpcorreia99

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Palantir"

@jpcorreia99

Copy link
Copy Markdown
Author

Karthik Nadig (@karthiknadig) little ping here

@jpcorreia99

Copy link
Copy Markdown
Author

Hey Karthik Nadig (@karthiknadig) are the other two reviews still necessary? I just tagged the people who I've seen contributing the most recently to the extension

@karthiknadig
Karthik Nadig (karthiknadig) merged commit 79e8a13 into microsoft:main Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants