cylc-flow icon indicating copy to clipboard operation
cylc-flow copied to clipboard

wrapper script: consider clearing PYTHONPATH

Open oliver-sanders opened this issue 2 years ago • 4 comments

We advise that Cylc is installed into an isolated environment (e.g. conda / virtualenv). Unfortunately the PYTHONPATH environment variable can corrupt these.

Illustrative example (less extreme cases can cause problems too):

export PYTHONPATH='/path/to/python3.6/site-packages'
cylc version
Traceback ... not quite so isolated after all

This is very inconvenient for users who rely on PYTHONPATH (e.g. to load their own Python environment) as they have to unload this environment before they can run Cylc commands. If Cylc is installed in an isolated environment, then it should be isolated from stuff like this.

We might be able to do something like this:

PYTHONPATH= exec "${CYLC_HOME}/bin/${0##*/}" "$@"

Otherwise, there is the nuclear [Python] option:

-E     : ignore PYTHON* environment variables (such as PYTHONPATH)

Probably a good idea? But it does have the unwelcome side-effect of disabling PYTHONBREAKPOINT.

And also:

-s     : don't add user site directory to sys.path; also PYTHONNOUSERSITE

Could do with a bit of investigation to work out what the consequences of these approaches are.

Pull requests welcome!

oliver-sanders avatar Sep 08 '22 11:09 oliver-sanders

(Tagged 8.1.0 but could go into 8.0.x if we can agree a solution in time)

oliver-sanders avatar Sep 08 '22 11:09 oliver-sanders

Note: Users might need PYTHONPATH to load modules for rose_ana which could make a mess of things.

oliver-sanders avatar Sep 08 '22 11:09 oliver-sanders

Unfortunately not everyone uses the wrapper! E.g. single user installations.

What about explicitly cleaning sys.path at start-up? I think that would work in conda environments. In a Python venv though, the system Python libs (used to create the venv) are needed in sys.path.

hjoliver avatar Sep 15 '22 05:09 hjoliver

I'm not sure that would work? The Python CLI options are probably a better bet.

Note that all Cylc users use a wrapper script, not necessarily the Cylc wrapper script, but the one generated for them by pypi or Conda so we might be able to change things there too.

Unfortunately I think we might actually need to bodge PYTHONPATH for rose_ana which makes a right mess of things.

oliver-sanders avatar Sep 15 '22 10:09 oliver-sanders

Note, this impacts Rose worse than Cylc, however, with Rose we actually need the PYTHONPATH to be preserved because it will be required for the Rose "command".

oliver-sanders avatar Aug 15 '23 15:08 oliver-sanders

Note, this impacts Rose worse than Cylc, however, with Rose we actually need the PYTHONPATH to be preserved because it will be required for the Rose "command".

Perhaps we need to save the PYTHONPATH in another environment variable which Rose can use before running the "command".

dpmatthews avatar Aug 16 '23 09:08 dpmatthews

That's sounding like the best option, note the variable would have to be set in the wrapper-script and restored in the rose library code.

oliver-sanders avatar Aug 21 '23 10:08 oliver-sanders

Suggestion, in the Cylc wrapper script:

  • Copy PYTHONPATH into a new var called _PYTHONPATH.
  • Wipe PYTHONPATH for the cylc/rose/whatever execution.
  • In rose, restore PYTHONPATH from _PYTHONPATH for app execution.

For context, here's a common practice:

# flow.cylc
[runtime]                                                               
    [[foo]]                                                             
        pre-script = module load foo    # load some dependencies of our app                            
        script = rose task-run    # <= wipe PYTHONPATH before this                                                                 
# rose-app.conf                                                         
[command]                                                               
default = foo   # <= restore PYTHONPATH before this   

oliver-sanders avatar Sep 04 '23 15:09 oliver-sanders

I think that I've done the rose end, but the wrapper script is proving trickier. We don't really want to cache PYTHONPATH every time we run the wrapper script - otherwise it risks stripping out the path we want for apps like cylc message?

wxtim avatar Sep 08 '23 15:09 wxtim