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

Ensure we block on autoselection when no interpreter is explictly set by user #16723

Merged
merged 3 commits into from
Jul 20, 2021

Conversation

karrtikr
Copy link

Will add tests once @kimadeline confirms this fixes the issue.

Copy link

@kimadeline kimadeline left a comment

Choose a reason for hiding this comment

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

It works! 🥳

I am hitting a second problem, although that might be by design:

  1. The first time updateDisplay is called the interpreter is python, so it will trigger auto-selection, so far so good;
  2. The auto-selection logic will query interpreterService.getInterpreters, and it will return only the cached interpreters, so the interpreter that will be displayed in the status bar is going to be one of my global interpreters;
  3. Then the auto-selection will be triggered again, this time by ExtensionActivationManager.activate, and this time the local virtual environment selected will be returned, but there is no updateDisplay call to reflect that change.

I assume 2. is by design, so do we want to do anything about it? I will file a separate issue: #16727

@karthiknadig
Copy link
Member

@kimadeline @karrtikr I think 2 is a problem but in the discovery code. For globals it should return cached. For locals it should return cached local environments, if there are none cached then it should looks for one and return anything it finds.

@karrtikr karrtikr marked this pull request as ready for review July 20, 2021 01:53
@karrtikr
Copy link
Author

karrtikr commented Jul 20, 2021

@karthiknadig For now to keep things simple we decided to ignore cache for both global and local envs the first time. If we have performance issues we can optimize it later.

@karrtikr karrtikr merged commit 0263703 into microsoft:release-2021.07 Jul 20, 2021
@karrtikr karrtikr deleted the bugbashfix branch July 20, 2021 17:52
karthiknadig added a commit that referenced this pull request Jul 22, 2021
* Fix 'Cannot read property 'resolveEnv' of undefined' error (#16677)

* Cherry pick fixes into release (#16686)

* Bump actions/setup-node from 2.1.5 to 2.2.0 (#16592)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 2.1.5 to 2.2.0.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.1.5...v2.2.0)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add support for starting TensorBoard session with a remote log directory via URL (#16477)

* Add support for remote logdirs

* Add missing keys

* Fix typo

* Bump isort from 5.8.0 to 5.9.2 (#16636)

Bumps [isort](https://github.com/pycqa/isort) from 5.8.0 to 5.9.2.
- [Release notes](https://github.com/pycqa/isort/releases)
- [Changelog](https://github.com/PyCQA/isort/blob/main/CHANGELOG.md)
- [Commits](PyCQA/isort@5.8.0...5.9.2)

---
updated-dependencies:
- dependency-name: isort
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make getInterpreters() API faster for subsequent calls (#16674)

* Change the way auto-selection works (#16644)

* New comparison logic

* Add experiment group

* Register and call it

* Add service registry tests

* Add interpreter selector unit tests

* Add comparison unit tests

* Add intepreter selector test

* News file

* Adjust comments

* Reuse getSortName

* Add new auto-selection logic

* Add tests for getEnvTypeHeuristic

* Move persistent store initialization back out

* Update tests

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

* Change version for release (#16722)

* Ensure we block on autoselection when no interpreter is explictly set by user (#16723)

* Ensure we block on autoselection when no interpreter is explictly set by user

* Added tests

* News entry

* Update change log for release. (#16731)

* Fix autoselection when opening a python file directly (#16733)

* Fix autoselection when opening a python file directly

* Update changelog

* Add tests

* Ignore cache when querying for interpreters during auto-selection (#16734)

* Ignore cache when getting envs for autoselection

* Don't call autoSelectInterpreter twice

* Update debugger via point release (#16746)

* Update version

* Update change log

* Update wheels to 3.9 (#16745)

* Clean up

Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
karthiknadig added a commit that referenced this pull request Jul 28, 2021
* Fix 'Cannot read property 'resolveEnv' of undefined' error (#16677)

* Cherry pick fixes into release (#16686)

* Bump actions/setup-node from 2.1.5 to 2.2.0 (#16592)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 2.1.5 to 2.2.0.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.1.5...v2.2.0)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add support for starting TensorBoard session with a remote log directory via URL (#16477)

* Add support for remote logdirs

* Add missing keys

* Fix typo

* Bump isort from 5.8.0 to 5.9.2 (#16636)

Bumps [isort](https://github.com/pycqa/isort) from 5.8.0 to 5.9.2.
- [Release notes](https://github.com/pycqa/isort/releases)
- [Changelog](https://github.com/PyCQA/isort/blob/main/CHANGELOG.md)
- [Commits](PyCQA/isort@5.8.0...5.9.2)

---
updated-dependencies:
- dependency-name: isort
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make getInterpreters() API faster for subsequent calls (#16674)

* Change the way auto-selection works (#16644)

* New comparison logic

* Add experiment group

* Register and call it

* Add service registry tests

* Add interpreter selector unit tests

* Add comparison unit tests

* Add intepreter selector test

* News file

* Adjust comments

* Reuse getSortName

* Add new auto-selection logic

* Add tests for getEnvTypeHeuristic

* Move persistent store initialization back out

* Update tests

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

* Change version for release (#16722)

* Ensure we block on autoselection when no interpreter is explictly set by user (#16723)

* Ensure we block on autoselection when no interpreter is explictly set by user

* Added tests

* News entry

* Update change log for release. (#16731)

* Fix autoselection when opening a python file directly (#16733)

* Fix autoselection when opening a python file directly

* Update changelog

* Add tests

* Ignore cache when querying for interpreters during auto-selection (#16734)

* Ignore cache when getting envs for autoselection

* Don't call autoSelectInterpreter twice

* Update debugger via point release (#16746)

* Update version

* Update change log

* Update wheels to 3.9 (#16745)

* Point release with debugpy fix (#16776)

Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants