Skip to content

Fall back when Python environments API is incomplete - #81

Open
sama Pyb (Pybsama) wants to merge 1 commit into
microsoft:mainfrom
Pybsama:codex/fallback-incomplete-python-environments-api
Open

sama Pyb (Pybsama) wants to merge 1 commit into
microsoft:mainfrom
Pybsama:codex/fallback-incomplete-python-environments-api

Conversation

@Pybsama

Copy link
Copy Markdown

Summary

  • validate the preferred Python Environments API's required runtime capabilities before wrapping or caching it
  • fall back to the legacy Python Extension API when the preferred API returns undefined or incomplete exports
  • keep onDidChangePackages optional and provide a no-op disposable when that non-critical event is unavailable

Context

When the Python Environments integration is disabled, the installed extension can return undefined exports without throwing. The shared provider previously cached that result and later failed while subscribing to environment/package events instead of trying its existing legacy fallback.

This change belongs in the shared package consumed by Pylint and the other VS Code Python tool extensions. It does not bump the package version or update downstream submodules; the existing release workflow owns those steps after merge.

Addresses microsoft/vscode-pylint#856.

Testing

  • TypeScript test suite: 134 passing
  • TypeScript build
  • ESLint
  • Python test suite: 333 passing on Python 3.10
  • Python flake8, Black, and isort checks
  • version consistency validation

@Pybsama

Copy link
Copy Markdown
Author

Revalidated this change against current main at f839a70789f28ccd96f428d51685d60f07b5189c. The six commits since the PR base do not touch either PR file; the PR applies without conflicts, and the exact current-main merge tree is 6298a54342c9d9389d1fed8929d3dd54cfa1bb6f.

The runtime case remains relevant: current main still pins @vscode/python-environments 1.0.0, whose helper returns extension.exports without validating it.

Current-main validation:

  • removing only the capability guard/no-op fallback makes all 3 focused regressions fail; restoring the patch gives 3/3, plus 100/100 consecutive focused repetitions
  • Node 24.17.0: 134/134 TypeScript tests, test build, package build, and ESLint passed
  • CPython 3.10.20: 334/334 tests; Flake8, Black, isort, version consistency, and dependency checks passed
  • package dry run: 58 entries; compiled dist/python.js contains the capability guard and optional-event fallback
  • git diff --check: passed; the patch remains 2 files with no dependency or version changes
  • npm run format-check reports the same 14 pre-existing source warnings on untouched current main and the merge tree; that script is not part of the PR workflow

The upstream run now has 19 successful checks, including Linux/Windows Python 3.10–3.14, TypeScript, both CodeQL analyses, version checks, and CLA. The only failure is Ensure Required Labels, whose annotation requires one of bug, debt, dependencies, feature-request, or no-changelog; the repository reports my permission as READ, so a maintainer needs to apply the appropriate label.

The public head remains the original GitHub-verified commit because GitHub reports the PR as mergeable and the current-main changes do not overlap the patch. Marking this ready for maintainer review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation addresses the reported failure with focused, comprehensive tests and no unresolved issues.

Pull request overview

Adds robust fallback behavior when the preferred Python Environments API is unavailable or incomplete.

Changes:

  • Validates required API capabilities before caching.
  • Falls back to the legacy Python API.
  • Treats package-change events as optional with a no-op disposable.
File summaries
File Description
typescript/src/python.ts Implements API validation, fallback, and optional package events.
typescript/tests/python.test.ts Tests fallback, caching, and missing optional events.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@edvilme

Copy link
Copy Markdown
Contributor

sama Pyb (@Pybsama)
Hello! Thank you for your fix! We have made some changes since your last commit on both the environments extension and this repo. If you have the time to solve the merge conflicts so we can review, we would appreciate that greatly! If not, no worries, we will take care of that. Thanks for your contribution!

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