Use 'conda run' when debugging user code

[kimadeline]

Work item associated to the spike #8421

[karrtikr]

To make this work,

• LaunchRequestArguments.pythonPath is deprecated in the debugger. Remove all occurences of "pythonPath" is the extension.
• Check conda version to make sure conda run command is supported.
• If the current interpreter is conda, fetch two things.
  • <path to conda.exe> : Use condaService.getCondaFile() to do that.
  • Name and path of the conda environment : Use condaService.getCondaEnvironment() to fetch that.
    Now add this to the launch configuration in the debug resolver,

{
    "name": "Python: Current File",
    "type": "python",
    "request": "launch",
    "program": "${file}",
    "console": "integratedTerminal",
    "python": ["<path to conda.exe>", "run", "-n","<name of environment>", "python"] // <--- This is what you will have to add if conda environment has a name
}

OR

{
    "name": "Python: Current File",
    "type": "python",
    "request": "launch",
    "program": "${file}",
    "console": "integratedTerminal",
    "python": ["<path to conda.exe>", "run", "-p","<path to environment>", "python"] // <--- This is what you will have to add if conda environment has does not have name
}

• If the current interpreter is not conda, you have to add "python": <path to the selected python interpreter> in the launch resolver. This is because debugger expects 'python' must have at least 1 element.
• In earlier versions, one could use double dashes in conda run command to separate CLI flags for clarity. Something like,

conda run -n base -- python <...>`

But it seems support for that has been deprecated in conda. However one could still use

conda run -n base python -- <...>`

as python is able to parse double dashes.

• Edit package.json to support intellisense for "python" in launch.json. Also make sure to remove intellisense for "pythonPath".

[DonJayamanne]

If the current interpreter is conda, fetch two things

Please ensure we re-use existing code instead of hardcoding logic in other places.
As it is, we have 3 rules (if cond, if env name, if env path) in this place and we have missed one crucial rule (checking version of conda). Hence the need to re-use code.
@kimadeline @karthiknadig /cc

[int19h]

If the current interpreter is not Conda, the extension should provide the full path to the corresponding Python binary in "python", just like it did before via "pythonPath".

In the common case of a single value, it can be specified directly - i.e. "python": "foo" is the same as "python": ["foo"]

But note that "python" is a ptvsd 5 thing, so none of this should kick in if the older version is in use - it should continue to use "pythonPath".

[karrtikr]

@int19h Oh right. Edited the issue accordingly.
@DonJayamanne I added that we need to check conda version only to check if conda run is supported.

[DonJayamanne]

Still unsure why we're documenting an existing logic. what if another is missed again.
Also this solution implies we write this code again, instead of re-using... Anyways, thats my opinion .

[karrtikr]

Oh I see what you're saying.
Was just documenting to make sure we don't forget about it. We'll definitely try to re-use existing logic where we can.

[karthiknadig]

Make sure that when we test this, we also test it with auto-activate terminal setting turned on.

[DonJayamanne]

Shouldn't we use conda activate then run python code, instead of conda run?
I think conda run would work when not running in a terminal, but when in a terminal conda activate would be better

Else user output won't be displayed in terminal, input will not work, etc..

[luabud]

@DonJayamanne this is for calling the debugger, not running python code

[DonJayamanne]

calling the debugger, not running python code

But with the way debug adapter is designed, why do we even need to run the adapter with conda run!
The adapter doesn't run any user code? So it's not necessary at all.
It's the process that's launched by the adapter that needs conda environment

@int19h @karthiknadig /cc

[int19h]

That's exactly what the proposal does: "python" is a property that gets parsed by the adapter; or rather by the launcher, which is spawned by the adapter via "runInTerminal" request to VSC. The launcher then applies it when spawning the debuggee. Neither the adapter nor the launcher run in the activated environment.

Are you saying that the debuggee needs to be spawned using conda activate rather than conda run, due to conda/conda#8386? It looks like they have fixed the issue with redirection recently, so I don't know if we have to worry about that anymore.

If we do, I'm not sure how we can pull that off. We'd need to issue conda activate first as a separate "runInTerminal" request, but the problem with those is that VSC sends the response to it as soon as the command starts running - there's no way for them to tell when it actually completes, though. So if we just send two requests sequentially, I don't think that'll do the right thing.

[DonJayamanne]

so I don't know if we have to worry about that anymore.

Isn't that applicable only if users have that version?

If not, something as simple as x = input() will not work in the terminal.
input is crucial, and this was the primary driver for changing the debugger default from debug console to terminal.

Also, when debugging apps like flask, or simple console apps users expect to see the output (see the port/address). Yes such output is displayed in the debug console as well. However when debugging we display the terminal as the default. Hence none of this information is displayed when using conda run. Hence user see nothing! I.e. today they see all the output, tomorrow with a new version, gone! 😄

Long story short, using conda run can change the debugger workflow for users & in some cases break it (no support for input). I.e. this is a change in UX.

We'd need to issue conda activate first as a separate "runInTerminal" request, but the problem .... I don't think that'll do the right thing.

Agree.

I believe we'll need to review the options, pros and cons and impact on UX. @luabud @karthiknadig @int19h

[DonJayamanne]

@DonJayamanne this is for calling the debugger, not running python code

@luabud Thanks, however the title seems to indicate this is for running user code. Or have i misunderstood your comment.

[DonJayamanne]

@int19h @karthiknadig
Something we could do is:

• DAP sends a custom message to extension
• Extension then opens a terminal (or re-uses terminal) & activates, finally runs user code.

Note:

• Run in terminal request doesn't support multiple commands. It only supports a single command. Commands such as activate & then run python code are two commands and this cannot be done with run in request.

I'll try to find the original issue in PVSC and VSC repo where this was captures.

Previous discussions on this same solution (discussed with upstream VSC):

• Debugger commands sent to terminal get swallowed by previous terminal command
• Other related issues here Conda Issues #5344

[int19h]

They can be done with two "runInTerminal" requests; the problem rather is that there's no way to wait for the first one to complete before issuing the second.

But I don't think the extension can handle it any better. The fundamental problem is that activation is a separate command, and there's no way with the VSCode terminal API to determine when command finishes execution. And that's because they're basically just sending the command as terminal input, so there's no way to determine when shell starts and finishes executing it (or even if it does).

So, the question rather is: how does the activation code knows that activation is complete, and it is safe to execute further commands?

Having a way to execute two commands with a single API call / DAP request would be ideal here. The proposed workarounds in microsoft/vscode#67692 are not great - they effectively require us to reimplement all the shell-specific argument quoting that VSC already does for all supported shells. It would be much easier for them to accept multiple commands, using the existing quoting logic, and just adding && or equivalent support per shell.

[DonJayamanne]

So, the question rather is: how does the activation code knows that

VSCode can do this:
conda activate xxx && python --version

I.e. chain the commands into one line.

[int19h]

You can do this with terminal.sendInput(), but that one is a low-level API that just takes a string as is - the caller has to do all the quoting.

I can't find any high-level terminal APIs that do shell-specific quoting - like IExternalTerminalService or "runInTerminal" DAP - that provide for multiple commands.

[int19h]

To clarify, what I'd like to avoid is having to re-implement all of this:
https://github.com/microsoft/vscode/blob/f395cac4fff0721a8099126172c01411812bcb4a/src/vs/workbench/contrib/debug/node/terminals.ts#L79-L210

[DonJayamanne]

To clarify, what I'd like to avoid is having to re-implement all of this:

I agree, there's more to this (keeping track of terminals, etc). And that's something we wanted to avoid in core extension team (back when I was there). @brettcannon is aware of these discussions.

[DonJayamanne]

@int19h Had a chat with @karthiknadig about an alternative. Creating a terminal with the right environment variables. This might help https://github.com/microsoft/vscode-python/issues/8928

[karrtikr]

Based on chats with the team:

• The gist of what we need to do is set the python field in the launch configuration to the

  [ "conda" , "run", "-n", "env_name", "--no-capture-output", "python"]
  

  and set the adapterPython to whatever python executable we want.

• This means it'll also use this command to run the launcher, unless we specify "debugLauncherPython", so it will show up as "conda run" in the terminal, which is what the users might expect. However note it does mean that we'll spawn "conda run" from inside another "conda run", which is very recently fixed Running using conda run inside already activated environment should be the same as running it outside conda/conda#11305.

• So we are waiting on conda to let us know their support timelines based on we can start using this approach.

With #20651 we're now using conda run to get environment variables in cases where some other python is explicitly specified by user. We should remove this with this issue.

[karrtikr]

We're currently getting env variables using conda run and applying to debugging or executing #20651, which also has been working well. Hence closing as not required for now.

[brettcannon]

We're currently getting env variables using conda run and applying to debugging or executing #20651, which also has been working well. Hence closing as not required for now.

And I assume this won't be a concern when debugging is launched from the Python Debugger extension or if people turn off the terminal activation feature?

[karrtikr]

That's right, we intercept and add the env variables independently of the experiment.