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

Always use environment path when running conda environment commands #24807

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jpcorreia99
Copy link

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
Author

@eleanorjboyd
@karthiknadig
@anthonykim1

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

@karthiknadig karthiknadig self-assigned this Feb 11, 2025
@karthiknadig karthiknadig added the bug Issue identified by VS Code Team member as probable bug label Feb 11, 2025
karthiknadig
karthiknadig previously approved these changes Feb 11, 2025
@vs-code-engineering vs-code-engineering bot added this to the February 2025 milestone Feb 11, 2025
@karthiknadig
Copy link
Member

@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.

@karthiknadig karthiknadig enabled auto-merge (squash) February 11, 2025 15:29
auto-merge was automatically disabled February 11, 2025 15:36

Head branch was pushed to by a user without write access

@jpcorreia99
Copy link
Author

jpcorreia99 commented Feb 12, 2025

hey @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
Author

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

@jpcorreia99
Copy link
Author

@karthiknadig little ping here

@karthiknadig karthiknadig enabled auto-merge (squash) February 13, 2025 19:39
@jpcorreia99
Copy link
Author

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

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.

2 participants