DIRACGrid / DIRAC

DIRAC Grid
http://diracgrid.org
GNU General Public License v3.0
113 stars 176 forks source link

comma in configuration is replaced with space, breaking pilot config option #7693

Closed maxnoe closed 4 months ago

maxnoe commented 4 months ago

The DIRAC pilot expects additional user environment variables in the --userEnvVariables option as comma separated list of key:::value pairs.

However, when setting values in the dirac configuration like this:

ExtraPilotOptions = --userEnvVariables DIRACSYSCONFIG:::pilot.cfg,RUCIO_HOME:::/home/dirac/rucio

What actually reaches the pilot has the comma replaced by a space:

Executing: python3 dirac-pilot.py -S DPPS-Tests -l CTA -e CTA -N dirac-ce -Q normal -n CTAO.CI.de --wnVO=ctao.dpps.test -o '/LocalSite/SharedArea=/home/dirac' --userEnvVariables DIRACSYSCONFIG:::pilot.cfg RUCIO_HOME:::/home/dirac/rucio --CVMFS_locations=/
2024-06-25T14:43:14.406469Z DEBUG [PilotParams] Options list: [('-S', 'DPPS-Tests'), ('-l', 'CTA'), ('-e', 'CTA'), ('-N', 'dirac-ce'), ('-Q', 'normal'), ('-n', 'CTAO.CI.de'), ('--wnVO', 'ctao.dpps.test'), ('-o', '/LocalSite/SharedArea=/home/dirac'), ('--userEnvVariables', 'DIRACSYSCONFIG:::pilot.cfg')]

thus the environment variable is not being set for the pilot.

Any idea how to set multiple environment variables for the pilot via the dirac configuration?

fstagni commented 4 months ago

Thank you for the notification. I think that this fix, which was made in response to this discussion is to blame.

What I can suggest is to replace the , in DIRACSYSCONFIG:::pilot.cfg,RUCIO_HOME:::/home/dirac/rucio with a ; by patching the Pilot code. Would that be OK?

maxnoe commented 4 months ago

Wouldn't a semicolon be interpreted as "end of command" by the shell?

maxnoe commented 4 months ago

If patching the pilot code is an option, maybe the best would be to allow giving --userEnvVarable key:::value multiple times?

maxnoe commented 4 months ago

Thinking a bit more about it, maybe the simplest solution is to upgrade this option from something that has to be passed via ExtraPilotOptions to a configuration option directly, e.g. like this:

CEDefaults {
    PilotEnvVars = key1:::value1
    PilotEnvVars += key2:::value2
    PilotEnvVars += key3:::value3
}

that can then be passed correctly to the pilot cli option down the line?

maxnoe commented 4 months ago

Another alternative would be to allow quoting the argument and properly handle quoted text when splitting.

E.g.:

ExtraPilotOptions = --userEnvVariables "DIRACSYSCONFIG:::pilot.cfg,RUCIO_HOME:::/home/dirac/rucio"

and then making sure the code here https://github.com/DIRACGrid/DIRAC/pull/6602 treats quoted parts properly

Edit:

regex based solution (adapted from https://stackoverflow.com/a/2787979/3838691):

In [1]: import re

In [2]: options =  '-userEnvVariables "DIRACSYSCONFIG:::pilot.cfg,RUCIO_HOME:::/home/dirac/rucio", --other-option'

In [3]: CSV_PATTERN = re.compile(r''',\s*(?=(?:[^'"]|'[^']*'|"[^"]*")*$)''')

In [4]: CSV_PATTERN.split(options)
Out[4]: 
['-userEnvVariables "DIRACSYSCONFIG:::pilot.cfg,RUCIO_HOME:::/home/dirac/rucio"',
 '--other-option']
fstagni commented 4 months ago

I propose https://github.com/DIRACGrid/DIRAC/pull/7698, seems the simplest (but requires a DIRAC release).

maxnoe commented 4 months ago

Should we close this now that #7698 is merged? Or do you have a policy of leaving issues open until a fix is released?

Thanks a lot anyways for the quick resolution!