spyder-ide / spyder

Official repository for Spyder - The Scientific Python Development Environment
https://www.spyder-ide.org
MIT License
8.32k stars 1.61k forks source link

Add a way to read, display and manage the environment variables passed to the IPython console #3891

Open ccordoba12 opened 7 years ago

ccordoba12 commented 7 years ago

This is needed to make Spyder work correctly with Tensorflow without starting it from a terminal. For example see:

http://stackoverflow.com/questions/41344017/why-does-the-tensorflow-module-import-fail-in-spyder-and-not-in-jupyter-notebo


@andfoy, this is for 3.2, so please don't worry about it right now. I opened this issue so we don't forget to address it in the future ;-)

ccordoba12 commented 7 years ago

This new entry should be added to our project preferences too.

goanpeca commented 7 years ago

Whats the idea here? to have a standalone pop up widget/dialog for this (a 2 column QTableView) or embed this inside the preferences... or both or?

ccordoba12 commented 7 years ago

We could have both, I still haven't decided it. In any case, we already have an env vars viewer (in the Options menu of the Python console), we just need to populate it at startup.

ccordoba12 commented 1 year ago

Hey @OyiboRivers, could you describe in what situations you need this functionality?

Also, @mrclary, I think we could use get_user_environment_variables in our kernelspec to grab the user env vars declared in .bash_profile or .bashrc and add those to os.environ, right?

mrclary commented 1 year ago

The IPython console already inherits the user's system environment variables via kernelspec. All platforms can view the environment variables via Tools -> Current user environment variables..., or the IPython console's hamburger menu -> Show environment variables. Both contexts should reflect the user's system environment variables.

Windows has the additional feature to edit these in the Tools menu widget; macOS and Linux do not. To modify environment variables for macOS and Linux, the user must modify his shell startup directly (~/.bashrc or ~/.bash_profile, etc.). It should be possible to extend the editing capabilities to macOS and Linux, but that will require discussion with @spyder-ide/core-developers regarding UX.

OyiboRivers commented 1 year ago

Hey @ccordoba12 ,

Thank you for your interest in my feature request, "Automatic Import of System Variables in Spyder #20989." The reason behind my request was a specific issue I encountered while using Spyder IDE in conjunction with a numba site-package in development mode in a virtual environment.

In my development workflow, I rely on numba functions for their performance boost. However, I noticed that every time I started a new Python session in Spyder and imported the site-package, numba would recompile and cache the entire package. This process resulted in a significant delay of ~144s at the start of a session, which was quite inconvenient and time-consuming.

After investigating the issue, I suspected that it might be related to the writing rights or the cache directory configuration of numba. To overcome this problem, I needed a way to specify a specific folder where numba could store the cached package. I needed a persistent virtual environment variable. One common approach is to set system variables in the bash profile to accessing them in Python by using “os.environ“. This process works using the terminal when executing a script. However, when running scripts in Spyder's IPython console, I realized that there was no direct access to system variables via “os.environ“.

To address this limitation, I made the feature request for Spyder to automatically import system variables. By implementing this feature, Spyder users would have access to system variables within the IPython console, enabling them in general store system variables that should be persistent in virtual environments which could be a frequent use case.

Currently, the workaround I use involves setting a startup script in Spyder's preferences. However, having an automated way to import system variables would greatly simplify this process.

Thank you for your effort dealing with my request, and I appreciate your attention to improving the functionality and usability of Spyder IDE.

OyiboRivers commented 1 year ago

Hey @mrclary , Thank you for your effort. I have tried to set system variables in both ~/.bashrc and ~/.bash_profile and tested scripts in the terminal and using the IPython console. I can run a python script with system variables using “os.environ“ via the terminal. No problem there. I have seen “Current user environment variables…“ in Spyder. The user defined variables do not show up and they can not be found in os.environ using the Ipython console either. As a workaround I am loading them into “os.environ“ via script on startup. If the IPython console inherits the user's system environment variables already then maybe something in my system settings prevents access to these variables. Unfortunately, I have no idea what to look for. Do you have any advice?

ccordoba12 commented 1 year ago

The IPython console already inherits the user's system environment variables via kernelspec

@mrclary, that works if you start Spyder in a terminal, but not if you do it graphically (i.e. from Gnome or KDE launchers). In that case, the environment variables passed to the kernel are be very few.

And that's because we're using os.environ instead of get_user_environment_variables in our kernelspec:

https://github.com/spyder-ide/spyder/blob/e09b7685e4872655349ac76cc122810618dd46f3/spyder/plugins/ipythonconsole/utils/kernelspec.py#L170-L174

But besides that, I think we need to improve how that function is working because right now is not picking up what you have in ~/.bashrc nor ~/.bash_profile.


To address this limitation, I made the feature request for Spyder to automatically import system variables

I have tried to set system variables in both ~/.bashrc and ~/.bash_profile and tested scripts in the terminal and using the IPython console. I can run a python script with system variables using “os.environ“ via the terminal. No problem there. I have seen “Current user environment variables…“ in Spyder. The user defined variables do not show up and they can not be found in os.environ using the Ipython console either.

@OyiboRivers, I understand your problem and how you tried to solve it perfectly now. Thanks a lot for taking the time to describe it in detail, that's really helpful for us.

We'll try to address this in Spyder 6, to be released before the end of the year.

OyiboRivers commented 1 year ago

Thank you @ccordoba12 and Team Spyder.

mrclary commented 1 year ago

@mrclary, that works if you start Spyder in a terminal, but not if you do it graphically (i.e. from Gnome or KDE launchers). In that case, the environment variables passed to the kernel are be very few.

I have confirmed that for our conda-based installers, launching Spyder graphically does not inherit the user's environment variables. This is an oversight on my part. I've configured menuinst to get user environment variables for macOS and this can be done for Linux as well.

Alternatively, we could explore using get_user_environment_variables to update kernelspec, as you suggest.

But besides that, I think we need to improve how that function is working because right now is not picking up what you have in ~/.bashrc nor ~/.bash_profile.

get_user_environment_variables uses the -l (login) flag which should cause sourcing ~/.bash_profile. This is the case for me on my Ubuntu VM. If ~/.bash_profile is not being sourced for you, then we need to figure out what is going wrong there.

It is expected that ~/.bashrc will not be sourced since the subprocess will not be an interactive shell. The -i (interactive) flag can be used to cause sourcing ~/.bashrc. However, we can only choose one or the other; using both flags (regardless of order) results in only sourcing ~/.bash_profile. For this reason it is common practice for a user to put if [ -f ~/.bashrc ]; then . ~/.bashrc; fi (or similar) in ~/.bash_profile (see https://www.gnu.org/software/bash/manual/html_node/Bash-Startup-Files.html).

mrclary commented 1 year ago

@ccordoba12, everything appears to behave as expected for me on my Ubuntu VM:

Now, if I include sourcing ~/.bashrc in either ~/.bash_profile or ~/.profile, get_user_environment_variables still does not pick up those variables. Why? I think I found the reason. My ~/.bashrc file contains the boilerplate code at the top

case $- in
    *i*) ;;
      *) return;;
esac

which will leave the script if it is not an interactive shell.

ccordoba12 commented 1 year ago

@mrclary, thanks for digging deeper into this problem! I can see the same code in the ~/.bashrc automatically created for new users in my system (and a similar one for my own user, which I've had for a long time).

So, I think that leaves us with one option: using an interactive shell in get_user_environment_variables. As I mentioned in our dev meeting, I think I managed to implement that. I'll upload a PR with my proposed changes after releasing alpha1 so you can take a look at them.

mrclary commented 1 year ago

I have not encountered any real issues when using the -i flag in get_user_environment_variables on my Ubuntu VM. The stderr of run_shell_command does return 'bash: cannot set terminal process group (2478): Inappropriate ioctl for device\nbash: no job control in this shell\n' but the stdout still has the expected environment variables.

In order to source both ~/.bashrc and ~/.*profile, it seems to work to call run_shell_command separately for both -l and -i flags, feeding the results of the first into the second.

ccordoba12 commented 1 year ago

I have not encountered any real issues when using the -i flag in get_user_environment_variables on my Ubuntu VM.

I found that running get_user_environment_variables a second time causes Spyder to freeze. A simple way to do that is by trying to open our PPM (which calls get_user_environment_variables behind the scenes).

Could you confirm?

In order to source both ~/.bashrc and ~/.*profile, it seems to work to call run_shell_command separately for both -l and -i flags, feeding the results of the first into the second.

Ok, that's an interesting finding. What code are you using for that?

mrclary commented 1 year ago

I found that running get_user_environment_variables a second time causes Spyder to freeze. A simple way to do that is by trying to open our PPM (which calls get_user_environment_variables behind the scenes).

Could you confirm?

https://github.com/spyder-ide/spyder/blob/d8ae5c3198d0944a6d696e20881d183b8a3a0c16/spyder/utils/environ.py#L65-L72

I have changed line 67 to use the -i flag and have opened/closed the PPM and the environment variables viewer without issue. Also, I've manually executed get_user_environment_variables in the internal console.

Screenshot 2023-06-16 at 3 09 04 PM

In all cases, Spyder has not frozen and I confirmed that ~/.bashrc variables are present.

So we need to figure out what is causing your Spyder to freeze...

In order to source both ~/.bashrc and ~/.*profile, it seems to work to call run_shell_command separately for both -l and -i flags, feeding the results of the first into the second.

Ok, that's an interesting finding. What code are you using for that?

Changing the above code to:

shell = os.environ.get("SHELL", "/bin/bash")
env_var = {}
for flag in ['-l', '-i']:
    cmd = f"""{shell} {flag} -c "{sys.executable} -c \'import os; print(dict(os.environ))\'" """
    proc = run_shell_command(cmd, env=env_var, text=True)
    stdout, stderr = proc.communicate()
    env_var = eval(stdout, None)
ccordoba12 commented 1 year ago

In all cases, Spyder has not frozen and I confirmed that ~/.bashrc variables are present.

Ok, then I think you can solve this issue yourself and don't need the changes I made. So, please go ahead and do it.

So we need to figure out what is causing your Spyder to freeze...

Good point. Now that I remember, I have some code in my bashrc that asks for user input. So that's probably what's freezing Spyder.

mrclary commented 1 year ago

Good point. Now that I remember, I have some code in my bashrc that asks for user input. So that's probably what's freezing Spyder.

Hmm... perhaps we need to think about this. Since ~/.bashrc is intended for interactive shells, we should expect this to be a possibility, however rare. I wonder if there is a way to send an EOF to stdin on a timeout. Or at least use the timeout in Popen.communicate so it doesn't hang Spyder.

mrclary commented 1 year ago

I put read -p "..." in my ~/.bashrc and that doesn't seem to hang up Spyder. Do you know what could be causing it in your file?

ccordoba12 commented 1 year ago

This is the command I have that asks for input:

# For adding ssh keys with password to the session
ssh-add -K ~/.ssh/id_rsa &>/dev/null
mrclary commented 1 year ago

@ccordoba12, on my Ubuntu VM, I added the same line to my ~/.bashrc and Enter PIN for authenticator: was requested when opening a new terminal. However, Spyder still does not hang when executing get_user_environment_variables several times. I can't reproduce your hang. Could there be some other cause for this?

mrclary commented 1 year ago

Okay, now I've reproduced it. But this is real strange. It will hang, regardless whether user prompts are present or not, if Spyder is launched from a terminal and is not a background process.

I'm testing using Spyder's internal console with the command run_shell_command('/bin/bash -i -c "echo hello"')

I did not reproduce the error before because I was testing on Spyder launched from a shell script that set it in the background.

I've only found one other possible solution. Rather than using /bin/bash -i to source ~/.bashrc, we have to source it directly using eval and bypassing the interactive check: eval "$(cat ~/.bashrc | tail -n 10)". This seems to work without issues, but cannot be a good general solution.

@ccordoba12, what was the work that you had been doing on this?

mrclary commented 1 year ago

Okay, the above needs some correction. I've since rebuilt my dev environment and tried to reproduce the conditions where Spyder did not freeze as well as when it does freeze.

Launching Spyder via bootstrap.py from the latest master on my Ubuntu VM and running commands from Spyder's internal (Python) console (after from spyder.utils.programs import run_shell_command):

So maybe I've found a solution, even if I don't completely understand what the underlying issue is. So, what is the best way to implement this?

  1. Create a <shell>-user-env.sh script for each type of shell (bash, zsh, etc.) calling the system's /usr/bin/python3 and saving these scripts in spyder/utils. Advantage: don't have to create the script at Spyder runtime. Disadvantage: relies on system python; track multiple shell scripts in the repo.
  2. Create a user-env.sh script (in a tmp directory? or Spyder's config directory?) on Spyder startup, based on the user's shell environment variable, and call sys.executable instead of /usr/bin/python3. Advantage: rely on Spyder's runtime Python executable; don't track several shell scripts in repo. Disadvantage: must create the script at runtime.

What do @spyder-ide/core-developers think?

jitseniesen commented 1 year ago

I reproduced your observations on my Linux Debian machine. I don't know what the underlying issue is either.

Regarding your two options on how to proceed, it seems good not to have to rely on the system python (which may not be installed, or not at /usr/bin/python3). So if creating the script at runtime is not too hard, then try that.

mrclary commented 10 months ago

@ccordoba12, was this issue resolved by #21065? Or is there something else that needs to be done?

mrclary commented 5 months ago

@ccordoba12, I think this issue was resolved by #21065. Please reopen if you disagree.

ccordoba12 commented 5 months ago

Actually, I disagree because I think we need to give users a graphical way in Spyder to pass env vars to the IPython console.

At the moment users can only use the env vars they have declared in .bashrc.

mrclary commented 5 months ago

I think we need to give users a graphical way in Spyder to pass env vars to the IPython console.

At the moment users can only use the env vars they have declared in .bashrc.

Okay, so maybe something similar to what we do for PYTHONPATH, correct?

ccordoba12 commented 5 months ago

Yep, that's the idea. Perhaps we could take the dialog we show when users go to

Tools > Preferences > Current user environment variables

and add to it the ability to set new env vars.

After saving changes there, we could send those env vars to all open consoles and pylsp too.

mrclary commented 5 months ago

That sounds good! I would also recommend the following.

What do you think?

ccordoba12 commented 5 months ago

I really like all your suggestions. They will provide users with a very intuitive UI to handle env vars in Spyder.