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

Improvements to IEnvironmentActivationService to use hidden terminals #2614

Closed
DonJayamanne opened this issue Dec 5, 2019 · 5 comments
Closed
Assignees

Comments

@DonJayamanne
Copy link
Contributor

Today we use IEnvironmentActivationService to generate environment variables for activated environments. E.g. when running python code on behalf of user as a background process.

We do this today by activating the environment and capturing the environment variables.
For this we get the activation commands (using a default shell) and run it as a process.

A better approach is:

  • Create a hidden terminal
  • Get activation command and send to terminal
  • Dump variables to text file (instead of stdout).

Solves

  • unicode issues (today scraping stdout unicode)
  • Activates in terminal (thats what some activation command expect, i.e. to be run within a shell).

Discussed with @karthiknadig (helps DAP) @int19h /cc

FYI - I have started some work on this.

@int19h
Copy link
Contributor

int19h commented Dec 5, 2019

I think this could even skip the terminal altogether, and just spawn the activation script directly (all it needs is to ensure that it runs under the same shell that the actual terminal will be).

@DonJayamanne
Copy link
Contributor Author

DonJayamanne commented Dec 6, 2019

I think this could even skip the terminal altogether, and just spawn the activation script directly (all it needs is to ensure that it runs under the same shell that the actual terminal will be).

Agreed. But using the terminal is easier as we don't need to worry about finding paths to shells.
The activation scripts will run in the exact same environment that would other wise run when users run their code. We also have the added advantage of displaying the shell for diagnostic purposes if something goes wrong.

Also this ensures startup scripts are correctly loaded in the shells (something conda requires and initializes when using conda init).

On a side note, a tty interface is required by either pipenv or pyenv. Can't remember which one it was.

@DonJayamanne DonJayamanne self-assigned this Jan 22, 2020
@rchiodo
Copy link
Contributor

rchiodo commented Feb 3, 2020

I believe this is working. At least it does for me. @IanMatthewHuff didn't you say you had a problem with activation with a new environment?

@IanMatthewHuff
Copy link
Member

@rchiodo no. My issue was different, I'm pretty sure, my issue was with our saving of the last successful jupyter environment I think.

@rchiodo
Copy link
Contributor

rchiodo commented Feb 3, 2020

Thanks. Validated then.

@rchiodo rchiodo closed this as completed Feb 3, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 10, 2020
@microsoft microsoft unlocked this conversation Nov 14, 2020
@DonJayamanne DonJayamanne transferred this issue from microsoft/vscode-python Nov 14, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants