Skip to content

Conversation

@rsarm
Copy link
Contributor

@rsarm rsarm commented Jan 28, 2021

closes #1693

@rsarm rsarm added this to the ReFrame sprint 21.02 milestone Jan 28, 2021
@rsarm rsarm requested review from teojgo and vkarak January 28, 2021 08:16
@rsarm rsarm self-assigned this Jan 28, 2021
@rsarm
Copy link
Contributor Author

rsarm commented Jan 28, 2021

@jenkins-cscs retry dom

Co-authored-by: Vasileios Karakasis <vkarak@gmail.com>
@vkarak vkarak changed the title [bugfix] Split options passed through -J to get the length of the scheduler flag [bugfix] Fix splitting of single char options passed through -J to get the length of the scheduler flag Jan 28, 2021
@vkarak vkarak changed the title [bugfix] Fix splitting of single char options passed through -J to get the length of the scheduler flag [bugfix] Fix handling of single character options passed through -J Jan 28, 2021
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rsarm I tested the PR and I verified what I pointed out in the issue passing -A=foo is wrong. It's not correct under POSIX and -correctly- Slurm rejects it:

[reframer060@reframe ~]$ srun -ptotal -A reframers hostname
reframe-1
[reframer060@reframe ~]$ srun -ptotal -A=reframers hostname
srun: error: Unable to allocate resources: Invalid account or account/partition combination specified

Should we then treat the -J A=foo option specially and pass -Afoo or -A foo?

@rsarm
Copy link
Contributor Author

rsarm commented Jan 28, 2021

@vkarak oh sorry! I was convinced that -A=foo worked!

I would prefer to do not support translating non-valid options like -A=foo to valid ones. I would expect the user to pass options exactly as they would be set on the scheduler script. On the other hand it's very simple to support it, I guess we can do this

        for opt in options.job_options:
            optstr, valstr = re.split(r'=|\s+', opt, maxsplit=1)
            if opt.startswith('-') or opt.startswith('#'):
                parsed_job_options.append(opt)
            elif len(optstr) == 1:
                parsed_job_options.append(f'-{optstr} {valstr}')
            else:
                parsed_job_options.append(f'--{optstr} {valstr}')

@vkarak
Copy link
Contributor

vkarak commented Jan 28, 2021

What I'm proposing I think it's on the same line with the conversion from -J account=foo to --account=foo. To me it seems valid to have a conversion such as -J A=foo to -A foo. The lhs is our syntax. Otherwise, the user would have to quote the argument -J 'A foo' which is not consistent with the account=foo and also not so nice to write. I would vote for supporting just this syntax then.

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@vkarak vkarak changed the title [bugfix] Fix handling of single character options passed through -J [bugfix] Fix handling of single character scheduler backend options passed through -J Jan 29, 2021
@vkarak vkarak merged commit eff16b0 into reframe-hpc:master Jan 29, 2021
@rsarm rsarm deleted the bugfix/joboptions branch March 10, 2021 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Single-char scheduler options passed through -J are not handled properly

2 participants