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

Validate Python interpreter version for native server #516

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jul 5, 2024

Summary

I'm not sure why this check was removed looking back at #431, but this validation should be done for both native server and ruff-lsp. Both uses a Python script which requires Python 3.7 or later and ruff in general also supports 3.7 and later.

Without this change, if someone were to use Python 3.6, the extension would give the following error:

2024-07-05 09:26:21.378 [error] Error while trying to find the Ruff binary: Error: Command failed: /Users/dhruv/.pyenv/versions/3.6.15/bin/python /Users/dhruv/work/astral/ruff-vscode/bundled/tool/find_ruff_binary_path.py
Traceback (most recent call last):
  File "/Users/dhruv/work/astral/ruff-vscode/bundled/tool/find_ruff_binary_path.py", line 33, in <module>
    ruff_binary_path = find_ruff_binary_path()
  File "/Users/dhruv/work/astral/ruff-vscode/bundled/tool/find_ruff_binary_path.py", line 20, in find_ruff_binary_path
    elif sys.platform == "darwin" and sys._framework:
AttributeError: module 'sys' has no attribute '_framework'

Test Plan

The above error still occurs and the fix for that is in a follow-up PR (#517).

Using the below settings:

{
  "ruff.nativeServer":true,
  "ruff.interpreter": ["/Users/dhruv/.pyenv/versions/3.6.15/bin/python"]
}

The logs shows that the version isn't supported:

2024-07-05 09:27:38.086 [error] Python version 3.6 is not supported.
2024-07-05 09:27:38.086 [error] Selected python path: /Users/dhruv/.pyenv/versions/3.6.15/bin/python
2024-07-05 09:27:38.086 [error] Supported versions are 3.7 and above.

@dhruvmanila dhruvmanila added the bug Something isn't working label Jul 5, 2024
@dhruvmanila dhruvmanila marked this pull request as ready for review July 5, 2024 03:59
src/extension.ts Outdated
Comment on lines 91 to 92
interpreter &&
interpreter.length > 0 &&
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
interpreter &&
interpreter.length > 0 &&
interpreter?.length > 0 &&

Copy link
Member

Choose a reason for hiding this comment

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

or does typescript complain that the left isn't a number. In that case I would write interpreter != null && interpreter.length > 0. We should probably enable the lint rule to enforce != instead of on truthiness checks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the compiler complains. I'll change it to use explicit null checks but I can't find the lint rule which corresponds to that in https://www.typescriptlang.org/tsconfig/.

Copy link
Member

Choose a reason for hiding this comment

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

@dhruvmanila dhruvmanila merged commit 50e9053 into main Jul 5, 2024
6 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/python-version-check branch July 5, 2024 11:36
dhruvmanila added a commit that referenced this pull request Jul 5, 2024
## Summary

This is a follow-up from #516 and adopted from
microsoft/vscode-python-tools-extension-template@723f297.

## Test Plan

Same test plan as #516 but validate that the script failure doesn't
occur now that we exit early.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants