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

Predictable ids for Conda envs without Python #20496

Closed
wants to merge 1 commit into from

Conversation

DonJayamanne
Copy link

Fixes #20176

fileName =
getOSType() === OSType.Windows ? path.join(env.envPath, fileName) : path.join(env.envPath, 'bin', fileName);
}
resolvedEnv.id = getEnvID(fileName, resolvedEnv.location);
Copy link
Author

Choose a reason for hiding this comment

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

I think this is a valid approach, because the new env will take the place of this old env.
& worst case scenario, the id will be different from the old env, which is already the case, hence no harm

@DonJayamanne DonJayamanne added skip tests Updates to tests unnecessary debt Covers everything internal: CI, testing, refactoring of the codebase, etc. labels Jan 12, 2023
@DonJayamanne DonJayamanne requested a review from karrtikr January 12, 2023 02:11
Copy link

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

I like the idea. But there're multiple places where we'll have to handle this change, making sure environment with old IDs are removed from cache, checking instances of getEnvID elsewhere etc.

It's probably better to holistically tackle it later if this change is not required atm, if it is though, let me know and I can reopen the PR.

@karrtikr karrtikr closed this Jan 12, 2023
@DonJayamanne
Copy link
Author

DonJayamanne commented Jan 12, 2023

It's probably better to holistically tackle it later if this change is not required atm, if it is though, let me know and I can reopen the PR.

This change is required, and makes it almost impossible to use conda envirronments without python.
To Jupyter extension this is a crucial change and not a debt item,
I'll update the comments (update - looks like I we have already had this conversation before and its also documented in the issue)

@DonJayamanne DonJayamanne reopened this Jan 12, 2023
@karrtikr
Copy link

Closing in favor of #20609.

@karrtikr karrtikr closed this Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Covers everything internal: CI, testing, refactoring of the codebase, etc. skip tests Updates to tests unnecessary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Environment Id is not static (it changes) when installing Python into an empty Conda environment
2 participants