Skip to content

Resolve environments using cache if cache has complete info - #17474

Merged
Kartik Raj (karrtikr) merged 3 commits into
microsoft:mainfrom
karrtikr:compelteinfo
Sep 21, 2021
Merged

Resolve environments using cache if cache has complete info#17474
Kartik Raj (karrtikr) merged 3 commits into
microsoft:mainfrom
karrtikr:compelteinfo

Conversation

@karrtikr

@karrtikr Kartik Raj (karrtikr) commented Sep 21, 2021

Copy link
Copy Markdown

Even if there is a refresh going on, we can rely on the cache if it has complete info.

@karrtikr Kartik Raj (karrtikr) changed the title Resolve environments using cache if cache has complete info regardless of whether there is a refresh going on Resolve environments using cache if cache has complete info Sep 21, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I understand the changes, but I don't understand the motivation behind it. Why aren't we using partial env info anymore?

@karrtikr

Copy link
Copy Markdown
Author

I understand the changes, but I don't understand the motivation behind it. Why aren't we using partial env info anymore?

resolveEnv is supposed to return complete info by definition, it is synonymous to getInterpreterDetails. So partial info in cache isn't useful.

Earlier we were relying on the fact whether a refresh is going on to see whether a cache has partial info, this check is more accurate.

// Envs in cache may have incomplete info when a refresh is happening, so
// do not rely on cache in those cases.
if (cachedEnv && this.refreshPromises.size === 0) {
if (cachedEnv) {

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.

Now that 3rd party extensions can also trigger a refresh, refresh can be going on any moment, we want to ensure we still use the cache in that case.

Comment thread src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts Outdated
@karrtikr
Kartik Raj (karrtikr) merged commit 688213e into microsoft:main Sep 21, 2021
@karrtikr
Kartik Raj (karrtikr) deleted the compelteinfo branch September 21, 2021 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip tests Updates to tests unnecessary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants