Skip to content

switch to optimistic + lazy registration for pipenv, pyenv and poetry managers#1423

Merged
eleanorjboyd merged 7 commits intomicrosoft:mainfrom
eleanorjboyd:zygotic-mosquito
Apr 2, 2026
Merged

switch to optimistic + lazy registration for pipenv, pyenv and poetry managers#1423
eleanorjboyd merged 7 commits intomicrosoft:mainfrom
eleanorjboyd:zygotic-mosquito

Conversation

@eleanorjboyd
Copy link
Copy Markdown
Member

most failures for pipenv/poetry/pyenv are at nativeFinderRefresh stage in startup. Most of these machines failing on these env manager are NOT using that manager. They're overwhelmingly venv users who just happen to have pipenv/poetry/pyenv installed on their system. Therefore this PR loads these three managers in an optimistic + lazy manner. Instead of trying to find if they exist on the machine at startup (which potentially causes PET to be called during manager registration) let them get registered. If they are registered, the search for environments will then be done later when the user hard-refreshes managers or clicks on the manager in the sidebar where at that point it will check for any envs of these manager types. It may find them (weirdly placed environments for any of these managers but PETs extensive finding succeeds) or not. If not the behavior is fine and just shows no data in the dropdown if no environments end up being found.

@eleanorjboyd eleanorjboyd added the bug Issue identified by VS Code Team member as probable bug label Apr 2, 2026
rzhao271
rzhao271 previously approved these changes Apr 2, 2026
Yoyokrazy
Yoyokrazy previously approved these changes Apr 2, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR changes the extension’s startup/registration flow for the pyenv, pipenv, and poetry environment managers to reduce activation-time failures by registering these managers unconditionally and deferring environment/tool discovery until the manager is actually initialized (e.g., user expands it in the sidebar or refreshes).

Changes:

  • Register pyenv/pipenv/poetry managers unconditionally and defer discovery to lazy initialize().
  • Remove native-finder fallback tool detection from getPyenv/getPipenv/getPoetry and rely on discovery during refresh instead.
  • Add new telemetry event MANAGER.LAZY_INIT and update startup-flow documentation accordingly.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/managers/pyenv/pyenvUtils.ts Removes native-finder fallback from getPyenv and simplifies failureStage.
src/managers/pyenv/pyenvManager.ts Adds lazy-init telemetry emission around first refresh.
src/managers/pyenv/main.ts Switches to unconditional manager registration (no tool precheck).
src/managers/poetry/poetryUtils.ts Removes native-finder fallback from getPoetry and simplifies failureStage.
src/managers/poetry/poetryManager.ts Adds lazy-init telemetry emission around first refresh.
src/managers/poetry/main.ts Switches to unconditional env + package manager registration.
src/managers/pipenv/pipenvUtils.ts Removes hasPipenvEnvironments and native-finder fallback from getPipenv.
src/managers/pipenv/pipenvManager.ts Adds lazy-init telemetry emission around first refresh.
src/managers/pipenv/main.ts Switches to unconditional manager registration (no tool/env precheck).
src/extension.ts Updates activation to call new register signatures without projectManager.
src/common/telemetry/constants.ts Adds EventNames.MANAGER_LAZY_INIT and GDPR mapping.
docs/startup-flow.md Updates documented activation flow to match lazy registration.
Comments suppressed due to low confidence (3)

src/managers/pyenv/pyenvManager.ts:124

  • The catch here swallows initialization failures (only classifies the error) without writing anything to the extension logs. Since lazy init now happens on-demand, missing a traceError/traceInfo with the exception makes user-reported failures much harder to diagnose. Consider logging the error (and any failureStage if present) before resolving _initialized.
            if (toolSource === 'none') {
                result = 'tool_not_found';
            }
        } catch (ex) {
            result = 'error';
            errorType = classifyError(ex);

src/managers/pipenv/pipenvManager.ts:123

  • Initialization errors are caught and converted into telemetry only, with no log output. Given this runs in response to user actions (expanding the manager / refreshing), please log the exception (e.g. via traceError) so failures aren’t silent in the output channel.
            if (toolSource === 'none') {
                result = 'tool_not_found';
            }
        } catch (ex) {
            result = 'error';
            errorType = classifyError(ex);

src/managers/poetry/poetryManager.ts:123

  • Like the other lazy managers, exceptions during initialize() are swallowed without any trace logging. Please log the caught exception (and optional failureStage) so Poetry discovery failures can be debugged from extension logs, not just telemetry.
            if (toolSource === 'none') {
                result = 'tool_not_found';
            }
        } catch (ex) {
            result = 'error';
            errorType = classifyError(ex);

@eleanorjboyd eleanorjboyd enabled auto-merge (squash) April 2, 2026 22:46
@eleanorjboyd eleanorjboyd merged commit 9a9e93a into microsoft:main Apr 2, 2026
16 checks passed
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.

5 participants