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

Add multiroot-workspace variable scoping for python.defaultInterpreterPath #18650

Closed
OmeGak opened this issue Mar 7, 2022 · 9 comments · Fixed by #19188
Closed

Add multiroot-workspace variable scoping for python.defaultInterpreterPath #18650

OmeGak opened this issue Mar 7, 2022 · 9 comments · Fixed by #19188
Assignees
Labels
area-environments Features relating to handling interpreter environments community ask Feature request that the community expressed interest in feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@OmeGak
Copy link

OmeGak commented Mar 7, 2022

It's currently not possible to use multiroot-workspace variable scoping to configure python.defaultInterpreterPath, as @kimadeline mentioned in #18207 (comment).

My use case is the following. I have multiple python projects in my workspace but I want them all to use the same virtualenv, existing under one of the workspace folders. The tree structure looks like this:

/
├── python-a/
│  ├── .venv/
│  └── foo.py
├── python-b/
│  └── bar.py

To make it work with python.defaultInterpreterPath I need to configure it like this in the workspace file:

"python.defaultInterpreterPath": "${workspaceFolder}/../python-a/.venv/bin/python"

This is fragile, as it will break if python-b/ workspace folder changes its absolute path. With multiroot-workspace variable scoping it would be possible to express this dependency more robustly like this:

"python.defaultInterpreterPath": "${workspaceFolder:python-a}/.venv/bin/python"
@OmeGak OmeGak added triage-needed Needs assignment to the proper sub-team feature-request Request for new features or functionality labels Mar 7, 2022
@karthiknadig karthiknadig removed the triage-needed Needs assignment to the proper sub-team label Mar 8, 2022
@karthiknadig karthiknadig added area-environments Features relating to handling interpreter environments triage labels Mar 8, 2022
@karrtikr karrtikr added investigating We are looking into the cause of the issue and removed triage labels Mar 8, 2022
@munro
Copy link

munro commented Mar 14, 2022

@OmeGak I think there may be a bit over overlap here with #18674 (please up vote 😄 )

I don't seem to have this issue specifically, but I have set this in my subworkspace's ./.vscode/settings.json:

{
    "python.defaultInterpreterPath": "${workspaceFolder}/.venv/bin/python"
}

@Samuel-Therrien-Beslogic

One of the projects I work with is a microservices monorepo
Each service (as well as the common/shared code library/module) have their own .venv
As a workaround I've created a virtual environment at the root of the project and pointed defaultInterpreterPath to it "python.defaultInterpreterPath": ".venv/bin/python", so everything is installed there.
And changed all my debug tasks commands to use the ${workspaceFolder} ("command": "cd backend/services/collect-service; poetry run ${workspaceFolder}/.venv/bin/python run.py",).

But this really isn't the best and is bound to have issues as soon as there's version conflicts between services.
It would be nice if VSCode devs didn't have to do that extra setup and could have scoped virtualenvs (which those using PyCharm don't have an issue with)

@karrtikr
Copy link

Note https://code.visualstudio.com/docs/editor/variables-reference#_variables-scoped-per-workspace-folder is currently only supported in tasks.json or launch.json (used while debugging), the use for it in settings.json is discouraged.

@OmeGak In this usecase .venv/ does not belong to the first workspace folder and is something the workspace file owns instead, so I'm wondering if the concept of ${workspaceFile} would work better instead. It can be used to access the path to the .code-workspace file.

@karrtikr karrtikr added needs community feedback Awaiting community feedback and removed investigating We are looking into the cause of the issue labels Apr 19, 2022
@karrtikr
Copy link

Thanks for the feature request! We are going to give the community 60 days from when this issue was created to provide 7 👍 upvotes on the opening comment to gauge general interest in this idea. If there's enough upvotes then we will consider this feature request in our future planning. If there's unfortunately not enough upvotes then we will close this issue.

@karrtikr karrtikr removed their assignment Apr 19, 2022
@OmeGak
Copy link
Author

OmeGak commented Apr 20, 2022

Thanks for your answer @karrtikr!

Note https://code.visualstudio.com/docs/editor/variables-reference#_variables-scoped-per-workspace-folder is currently only supported in tasks.json or launch.json (used while debugging), the use for it in settings.json is discouraged.

The vscode-python extension allows to substitute variables in the python.defaultInterpreterPath setting, however. I knew that variable substitution is only enabled built-in for a few setting keys in settings.json but I didn't know that this practice was discouraged for extensions. I'd be curious to read more info about it, if you have any link at hand. Is workspace scoping going too far?

@OmeGak In this usecase .venv/ does not belong to the first workspace folder and is something the workspace file owns instead

That is one way to look at it but in my use case .venv/ does belong to the python-a workspace. python-a is a full-fledged Python project. The other workspaces are either python-a plugins or python-a Git submodules that are installed in the same virtualenv as python-a and should use the same Python interpreter when I debug them.

I'm wondering if the concept of ${workspaceFile} would work better instead. It can be used to access the path to the .code-workspace file.

This is certainly another workaround. It's still more fragile than being able to use ${workspaceFolder:python-a}, as I will need to hardcode python-a path twice in the .code-workspace file.

"folders": [
    {
        "name": "python-a",
        "path": "projects/my-python-project/python-a"
    }
],
"settings": {
    "python.defaultInterpreterPath": "projects/my-python-project/python-a/.venv/bin/python"
}

Versus:

"folders": [
    {
        "name": "python-a",
        "path": "projects/my-python-project/python-a"
    }
],
"settings": {
    "python.defaultInterpreterPath": "${workspaceFolder:python-a}/.venv/bin/python"
}

@brettcannon
Copy link
Member

Thank you to everyone who upvoted this issue! Since the community showed interest in this feature request we will leave this issue open as something to consider implementing at some point in the future.

We do encourage people to continue 👍 the first/opening comment as it helps us prioritize our work based on what the community seems to want the most.

@brettcannon brettcannon added needs proposal Need to make some design decisions community ask Feature request that the community expressed interest in and removed needs community feedback Awaiting community feedback labels May 12, 2022
@karrtikr
Copy link

@OmeGak Apologies for the delayed response and thanks for the insight!

This is certainly another workaround. It's still more fragile than being able to use ${workspaceFolder:python-a}

Two workspace folders can share the same base name as well where this does not work in its own right. However https://code.visualstudio.com/docs/editor/variables-reference#_variables-scoped-per-workspace-folder also documents this, so maybe it's not considered such a practical scenario. We'll still probably go with ${workspaceFolder:python-a} to be consistent.

I didn't know that this practice was discouraged for extensions. I'd be curious to read more info about it, if you have any link at hand. Is workspace scoping going too far?

Supporting such system variables in settings.json were being a problem for LSP which is probably why they aren't supported by VSCode, IIRC. I'll see if I can find any links.

@karthiknadig
Copy link
Member

@OmeGak The reason settings.json substitutions are complicated because settings are hierarchical. In some cases there might not be a good way to resolve. Like in this scenario:
image

with this as a .code-workspace

{
    "folders": [
        {
            "path": "notpython" // not a python folder
        },
        {
            "path": "python1" // the python folder that contains .pylintrc
        },
        {
            "path": "python2" // the python folder does NOT contain .pylintrc
        },
        {
            "path": "." // root workspace parent of both python1 and python2
        }
    ],
    "settings": {
        "pylint.args": ["--rcfile=${workspaceFolder}/.pylintrc"]
    }
}

What is the right resolution here for .pylintrc for a python file under python2?

  • C:\GIT\linters\multiroot\multipython\python2\.pylintrc is incorrect because there is no such file under that workspace root, but is the correct substituion based on the variable.
  • C:\GIT\linters\multiroot\multipython\.pylintrc is the correct file, but for a file under python2 we can't find the right thing to substitute that variable.

The issue boils down to, settings are hierarchical so substitutions are not straight forward. What works is in each folder had a settings.json of its own. Like this:
image

And you would not need to have variables that require substitution there, at least variables like ${workspaceFolder}.

@karrtikr
Copy link

Hi all, feature should be out in the pre-release version of the extension, use the following to try it out:

image

@karrtikr karrtikr added needs PR and removed needs proposal Need to make some design decisions labels May 24, 2022
@karrtikr karrtikr added the verification-needed Verification of issue is requested label May 24, 2022
@karrtikr karrtikr added this to the May 2022 milestone May 24, 2022
@amunger amunger added the verified Verification succeeded label May 31, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-environments Features relating to handling interpreter environments community ask Feature request that the community expressed interest in feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
7 participants