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

make parsing jobid from sbatch output more robust #443

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

BenWibking
Copy link

This adds --parsable to the sbatch options when submitting a SLURM job. Then, when looking for the jobid returned by SLURM, it only examines the last line of output, which should conform to the description given in the documentation (https://slurm.schedmd.com/sbatch.html#OPT_parsable).

Fixes #441.

@BenWibking BenWibking marked this pull request as ready for review May 8, 2024 19:53
@FrankD412 FrankD412 added the Bugfix Discussion/Implementation of a solution to a bug (usually a PR) label May 9, 2024
Copy link
Collaborator

@jwhite242 jwhite242 left a comment

Choose a reason for hiding this comment

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

Ok, so only comment i think i have here is a slight change to the regex to ensure we can't match numbers that are part of other words (i.e. it finding the '3' in 'Stampede3'). So how about tweaking the regex to something like the following to ensure it can only find numbers attached to slurm's nominal sbatch output. Something like the following, with or without the named capture groups:

(?:Submitted batch job \b(?P<jobid>\d+)\b)

or maybe don't bother capturing the first part but require it to match:

Submitted batch job \b(?P<jobid>\d+)\b

Likely not worth the hassle to get lookahead/lookbehind constructs to filter out everything but the number as I think one of them isn't even supported in python?

@jwhite242
Copy link
Collaborator

Oh, and one more comment. Having the full string in the match means we shouldn't have to assume it's the last line either, so just using the regex.search iterator should find a match on any line; should be safer in case those plugins also allow output after the jobid (would hope not, but easy enough to guard against that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Discussion/Implementation of a solution to a bug (usually a PR)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jobs stuck in PENDING state but they've actually completed or are currently running
3 participants