sdf-cli Migration to slurmrest#13
Open
2Ryan09 wants to merge 30 commits into
Open
Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Implements a migration away from direct sacct/sacctmgr subprocess reads toward Slurm REST (slurmrest) using an autogenerated OpenAPI Python client, supporting containerization goals and reducing reliance on SLURM CLI binaries for read paths.
Changes:
- Added
SlurmrestClientand updatedrun_sacct/slurmdump/slurmimportflow to emit and ingest newline-delimited JSON job objects. - Updated facility hold-state reads to use
slurmrestassociation endpoints, while keepingsacctmgr modifyfor the (currently) non-migratable write path. - Added CI + Makefile steps and documentation for generating the OpenAPI client; updated tests accordingly.
Reviewed changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
modules/slurmrest.py |
Introduces a REST client wrapper + helpers to translate REST responses into importer-friendly job/hold-state data. |
modules/coact.py |
Switches job collection/import to REST + JSON line protocol; replaces association reads with REST calls. |
tests/test_slurm_import.py |
Adds behavioral test ensuring REST memory TRES gets an M suffix for correct downstream unit conversion. |
tests/test_node_allocation.py |
Updates facility hold-state tests to mock REST association responses instead of sacctmgr output parsing. |
.github/workflows/ci-cd.yaml |
Generates the OpenAPI client in CI prior to dependency install/test. |
Makefile |
Adds generate-client target for local OpenAPI client generation. |
pyproject.toml |
Adds local editable OpenAPI client source and dependency changes to support the migration. |
uv.lock |
Locks updated dependency set including the editable OpenAPI client and related packages. |
README.md |
Documents local OpenAPI client generation workflow. |
docs/slurmrest_migration.md |
Adds rationale and migration notes (including unit/format differences). |
import-jobs.sh |
Removes slurmremap step now that import path consumes job objects/JSON lines. |
.gitignore |
Ignores generated OpenAPI client output and .env. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+50
to
+51
| # Setup JWT token for REST client | ||
| os.environ["SLURMREST_JWT"] = "test_token" |
Comment on lines
+207
to
+209
| # Clean up JWT token | ||
| if "SLURMREST_JWT" in os.environ: | ||
| del os.environ["SLURMREST_JWT"] |
Comment on lines
+1
to
+3
| # Migration of `sacctmgr` and `sacct` Calls to `slurmrest` | ||
|
|
||
| Currently, `sdf-cli` runs `sacctmgr` and `sacct` CLI tools directly via `subprocess` in order to gather account association information, job accounting information, and toggling the number of nodes assigned to a facility in the event of an overage. In a step towards containerization and more robust interaction with SLURM, it is desired to migrate these CLI tool usages to `slurmrest` endpoints. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Documentation showing how the existing four calls to
sacctmgrandsacctcould be migrated to useslurmrest, a stepping stone towardssdf-clicontainerization.Refer to
docs/slurmrest_migration.mdfor thought process