PrincetonUniversity / slurmise / 28182266059

89%

push

github

fix: centralize slurm job id resolution in slurm.py (#82)

* wip: Req'd slurmid for slurm.get_slurm_job_sacct

* refactor(slurm): centralize job id resolution in slurm.py

Add get_current_job_id, resolve_job_id and split_job_id so all SLURM
specifics (env var names, id formats, sacct) live in slurm.py. Rename
the step_name parameter of parse_slurm_job_metadata to step_id to match
the CLI and the rest of the codebase. Drop the SLURM_JOBID env check in
get_slurm_job_sacct so post-mortem recording with an explicit job id
works outside of a SLURM job.

* fix: resolve slurm job id from environment before combining with step id

Closes #79. Passing --step-id without --slurm-id produced the literal
job id "None.<step>" and ran 'sacct -j None'. All call sites now use
slurm.resolve_job_id, which falls back to SLURM_JOB_ID/SLURM_JOBID
before appending the step id, and slurm.split_job_id instead of ad-hoc
string splitting. api.py no longer reads the environment directly.

Also fixes a latent TypeError in JobDatabase.update_missing_data, which
called parse_slurm_job_metadata with a step_id keyword that did not
exist; tests now mock get_slurm_job_sacct (the subprocess boundary)
instead of parse_slurm_job_metadata so id resolution and sacct parsing
are exercised for real.

* chore: Generalize tests

280 of 330 branches covered (84.85%)

Branch coverage included in aggregate %.

153 of 153 new or added lines in 9 files covered. (100.0%)

1996 of 2235 relevant lines covered (89.31%)

1.79 hits per line