Skip to content

Predictable ids for Conda envs without Python#20496

Closed
DonJayamanne wants to merge 1 commit into
microsoft:mainfrom
DonJayamanne:issue20176
Closed

Predictable ids for Conda envs without Python#20496
DonJayamanne wants to merge 1 commit into
microsoft:mainfrom
DonJayamanne:issue20176

Conversation

@DonJayamanne
Copy link
Copy Markdown

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
Copy Markdown
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 Code quality issues labels Jan 12, 2023
@DonJayamanne DonJayamanne requested a review from karrtikr January 12, 2023 02:11
Copy link
Copy Markdown

@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
Copy Markdown
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
Copy Markdown

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 Code quality issues 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