payu-org / payu

A workflow management tool for numerical models on the NCI computing systems
Apache License 2.0
18 stars 25 forks source link

Shell options when running userscripts and examples in config.rst #444

Open blimlim opened 2 weeks ago

blimlim commented 2 weeks ago

Following on from the discussion about exporting environment variables for use in userscripts, I've been playing around with running different userscripts and came across a small issue. The example in config.rst of a subcommand userscript: echo "some_data" > input.nml currently doesn't work.

Placing this in config.yaml

userscripts:
   archive: echo "some_data" > input.txt

ends up just printing "some_data > input.txt" to the output rather than writing to a file:

...
qsub -q copyq -P tm70 -l ncpus=1 -l mem=4GB -N pre-industria_c -l wd -j n -v PAYU_PATH=/g/data/hh5/public/apps/miniconda3/envs/analysis3-24.01/bin,PAYU_CURRENT_RUN=0,MODULESHOME=/opt/Modules/v4.3.0,MODULES_CMD=/opt/Modules/v4.3.0/libexec/modulecmd.tcl,MODULEPATH=/g/data/hh5/public/modules:/etc/scl/modulefiles:/apps/Modules/restricted-modulefiles/matlab_monash:/opt/Modules/modulefiles:/opt/Modules/v4.3.0/modulefiles:/apps/Modules/modulefiles -W umask=027 -l storage=gdata/access+gdata/hh5+gdata/tm70+gdata/vk83 -- /g/data/hh5/public/apps/miniconda3/envs/analysis3-24.01/bin/python3.10 /g/data/hh5/public/apps/miniconda3/envs/analysis3-24.01/bin/payu-collate
some_data > input.txt
Writing manifests/restart.yaml
...

I think this might be due to the default setting of shell=False used in the sp.check_call call in the run_userscript method though am not completely sure:

def run_userscript(self, script_cmd):
        # First try to interpret the argument as a full command:
        try:
            sp.check_call(shlex.split(script_cmd))

I'm wondering whether it's preferable to update run_userscript to make commands like the example from config.rst possible, or instead to update config.rst to clarify the types of commands which can be run?

I think having shell=True would make it easier to provide arguments like environment variables and storage flags when running userscripts, but it still should be possible otherwise.