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

Setting env in CommandFunction overrides PATH env var #1781

Open
niansong1996 opened this issue Nov 26, 2024 · 0 comments
Open

Setting env in CommandFunction overrides PATH env var #1781

niansong1996 opened this issue Nov 26, 2024 · 0 comments

Comments

@niansong1996
Copy link

According to the examples here, the field env can be used to pass in env variables:

By default, the function hides stdout and returns it at the end:

import submitit
function = submitit.helpers.CommandFunction(["which", "python"])  # commands must be provided as a list!
print(function())  # This returns your python path (which you be inside your virtualenv)
# for me: /private/home/jrapin/.conda/envs/dfconda/bin/python
Some useful parameters of the CommandFunction class:

cwd: to choose from which directory the command is run.
env: to provide specific environment variables.
verbose: set to False if you do not want any logging.
As an experimental feature, you can also provide arguments when calling the instance:

However, it seems that adding this field would lead to the execution failure due to the job unable to find the executable.

After some digging, I found that because we directly pass in env for the subprocess.Popen as in here:

with subprocess.Popen(
            full_command,
            stdout=subprocess.PIPE,
            stderr=subprocess.PIPE,
            shell=False,
            cwd=self.cwd,
            env=self.env,
        ) as process:

But according to the docs of Python, this could easily lead to such error as env will override all the env variables, including PATH:

Resolving the path of executable (or the first item of args) is platform dependent. For POSIX, see [os.execvpe()]
(https://docs.python.org/3/library/os.html#os.execvpe), and note that when resolving or searching for the executable path, cwd 
overrides the current working directory and env can override the PATH environment variable.

So I would suggest instead of simply doing evn=self.env, we should dump all the current env vars and only override the ones that presented in env. Or at least adding a warning in the examples to note that this could lead to overridden of other env vars like PATH.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant