fix: ci slurm check

[cmeesters]

This is a fix for breaking 3rd party workflows: Bioconda was unable to build (i.e. https://github.com/bioconda/bioconda-recipes/actions/runs/20041946577/job/57478140127?pr=61020) because an import of this plugin immediately threw an error due to the missing SLURM setup. This had repercussions in other repos, too.

Some bits of the test suite have been refactored.

Summary by CodeRabbit

• Bug Fixes

  • Detects absence of SLURM tools and disables the default status command to avoid misconfiguration when SLURM is not available.
  • Provides a clear user-facing message when SLURM is not detected and no status command can be used.

• Tests

  • Added focused parsing and time-conversion tests that run without SLURM present.
  • Removed older redundant tests and reorganized the test suite for clarity.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds a SLURM availability check to the plugin initializer so status-command autodetection returns None when SLURM tools are absent, and moves sacct parsing and time-to-seconds tests into a new dedicated test file while removing them from the previous test module.

Changes

Cohort / File(s)	Summary
SLURM availability detection snakemake_executor_plugin_slurm/__init__.py	Imported shutil; updated _get_status_command_default() to early-return None when shutil.which("sinfo") is falsy; _get_status_command_help() returns a message when default is None. Existing sacct/squeue fallback logic otherwise unchanged.
Test reorganization tests/test_parsing.py, tests/tests.py	Added tests/test_parsing.py with test_parse_sacct_data and extensive TestTimeToSeconds cases covering many sacct time formats and edge cases; removed those tests and related imports (including pandas) from tests/tests.py.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Check callers of _get_status_command_default() handle None and that _get_status_command_help() message is surfaced appropriately.
• Review new parsing/time tests for correctness and any required test fixtures (pandas usage).

Possibly related PRs

• refactor: first step to outsource query command #339 — Implements similar shutil.which("sinfo") SLURM presence check and early None return in status-command autodetection.

Suggested reviewers

• johanneskoester
• fgvieira

Poem

🐇 I sniffed the paths for sinfo's song,
When none was found I knew what belonged.
Tests hopped houses, tidy and bright,
Parsing time from day to night,
A little change, a lighter flight.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'fix: ci slurm check' is vague and does not clearly convey the main change—it refers to fixing a CI SLURM check, but the actual change is about preventing import errors when SLURM is not installed, which affects workflows and package building.	Consider a more descriptive title like 'fix: prevent SLURM import error when not installed' or 'fix: allow plugin import when SLURM is unavailable' to clearly communicate the key fix.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/ci_slurm_check

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ec5c30 and 9668ede.

📒 Files selected for processing (1)

• snakemake_executor_plugin_slurm/__init__.py (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• snakemake_executor_plugin_slurm/init.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: testing

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

snakemake_executor_plugin_slurm/__init__.py (1)

344-351: Handle None status_command when SLURM is unavailable.

When SLURM is not available (i.e., sinfo is not found), _get_status_command_default() returns None, which propagates through get_status_command() to check_active_jobs(). The code silently falls back to squeue without explicitly informing the user that SLURM is unavailable. Add an explicit check in check_active_jobs() to raise a clear WorkflowError when status_command_name is None, rather than allowing the executor to fail with a confusing error from squeue.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb09d7f and 372cd27.

📒 Files selected for processing (3)

• snakemake_executor_plugin_slurm/__init__.py (2 hunks)
• tests/test_parsing.py (1 hunks)
• tests/tests.py (0 hunks)

💤 Files with no reviewable changes (1)

• tests/tests.py

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 0
File: :0-0
Timestamp: 2025-01-13T09:54:22.950Z
Learning: PR #173 (adding gres resource specification) depends on PR #28 in snakemake-executor-plugin-slurm-jobstep repository, as changes were required in the cpu-settings function of the jobstep-Executor module.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 333
File: .github/workflows/announce-release.yml:0-0
Timestamp: 2025-07-22T17:31:41.629Z
Learning: In the snakemake-executor-plugin-slurm repository, cmeesters prefers using `main` references for GitHub Actions to always get the latest version, with security provided through protected branches and mandatory CI checks rather than commit SHA pinning.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 193
File: .github/workflows/post_to_mastodon.yml:26-26
Timestamp: 2025-01-20T09:13:26.443Z
Learning: In the snakemake-executor-plugin-slurm repository, release PRs follow the naming pattern "chore(main): release X.Y.Z" where X.Y.Z is the version number.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 187
File: .github/workflows/post_to_mastodon.yml:0-0
Timestamp: 2025-01-17T17:27:32.446Z
Learning: In the mastodon publishing workflow for snakemake-executor-plugin-slurm, the PR_TITLE environment variable is required by the post_to_mastodon.sh script and should be preserved.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 178
File: snakemake_executor_plugin_slurm/__init__.py:0-0
Timestamp: 2024-12-11T14:17:08.749Z
Learning: In the `snakemake-executor-plugin-slurm` project, when handling exceptions in `snakemake_executor_plugin_slurm/__init__.py`, prefer concise error messages and avoid unnecessary verbosity or exception chaining when it's not necessary.

Learnt from: johanneskoester
Repo: snakemake/snakemake-executor-plugin-slurm PR: 173
File: docs/further.md:96-96
Timestamp: 2025-03-10T15:20:51.829Z
Learning: PR #173 in snakemake-executor-plugin-slurm implements GPU job support by adding resources: `gres` for generic resource specifications (e.g., 'gpu:1'), `gpu`/`gpus` for specifying GPU counts, and `gpu_model`/`gpu_manufacturer` for specifying GPU types, allowing users to request GPU resources directly rather than having to use slurm_extra.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 173
File: snakemake_executor_plugin_slurm/__init__.py:29-31
Timestamp: 2025-02-18T14:53:09.504Z
Learning: The snakemake-executor-plugin-slurm tests require system dependencies (gcc, python3-dev) to build the datrie package, which is a dependency of snakemake.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 221
File: tests/tests.py:62-70
Timestamp: 2025-06-24T12:41:31.837Z
Learning: In the snakemake-executor-plugin-slurm efficiency report feature, the executor is responsible for creating the efficiency_report_path directory when generating efficiency reports. Tests should not manually check for directory existence but should rely on the executor to properly create necessary directories as part of its normal operation.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 249
File: snakemake_executor_plugin_slurm/submit_string.py:21-29
Timestamp: 2025-03-31T11:52:05.373Z
Learning: In the snakemake-executor-plugin-slurm project, account and partition parameters are validated in the submit function before reaching the get_submit_command function. By design, these parameters are either empty strings or already properly formatted with SLURM parameter tags, making additional validation in get_submit_command unnecessary.

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 159
File: snakemake_executor_plugin_slurm/utils.py:43-53
Timestamp: 2025-01-07T18:48:09.845Z
Learning: In the SLURM executor plugin, when cleaning up working directories after job completion, all empty directories must be deleted recursively, including nested empty directories, as partial cleanup is not sufficient.

📚 Learning: 2024-12-11T14:17:08.749Z

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 178
File: snakemake_executor_plugin_slurm/__init__.py:0-0
Timestamp: 2024-12-11T14:17:08.749Z
Learning: In the `snakemake-executor-plugin-slurm` project, when handling exceptions in `snakemake_executor_plugin_slurm/__init__.py`, prefer concise error messages and avoid unnecessary verbosity or exception chaining when it's not necessary.

Applied to files:

• snakemake_executor_plugin_slurm/__init__.py

📚 Learning: 2025-06-03T17:10:51.292Z

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 221
File: snakemake_executor_plugin_slurm/__init__.py:0-0
Timestamp: 2025-06-03T17:10:51.292Z
Learning: In the snakemake-executor-plugin-slurm repository, the create_efficiency_report method in snakemake_executor_plugin_slurm/__init__.py contains temporary testing/debugging code that logs directory contents using os.listdir() and directory paths. This code was inserted for testing purposes only and should be removed from the production implementation.

Applied to files:

• snakemake_executor_plugin_slurm/__init__.py

📚 Learning: 2025-09-15T12:00:31.977Z

Learnt from: cmeesters
Repo: snakemake/snakemake-executor-plugin-slurm PR: 355
File: snakemake_executor_plugin_slurm/submit_string.py:23-30
Timestamp: 2025-09-15T12:00:31.977Z
Learning: In the snakemake-executor-plugin-slurm project, the parameters params.run_uuid, params.slurm_logfile, and params.comment_str are always provided and never None, so validation for None values is not necessary for these specific parameters in the get_submit_command function.

Applied to files:

• snakemake_executor_plugin_slurm/__init__.py

🧬 Code graph analysis (1)

tests/test_parsing.py (1)

snakemake_executor_plugin_slurm/efficiency_report.py (2)

• parse_sacct_data (124-230)
• time_to_seconds (11-66)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: testing

🔇 Additional comments (4)

snakemake_executor_plugin_slurm/__init__.py (1)

13-13: LGTM!

The shutil import is appropriate for checking SLURM command availability and is correctly placed in alphabetical order.

tests/test_parsing.py (3)

39-146: Excellent comprehensive test coverage!

The TestTimeToSeconds class provides thorough coverage of the time_to_seconds function, including:

• Various SLURM time formats (D-HH:MM:SS, HH:MM:SS, MM:SS, SS)
• Fractional seconds for TotalCPU format
• Edge cases (empty, None, invalid, whitespace, pandas NA)
• Real-world examples from actual sacct output

This ensures the parsing logic works reliably across different SLURM output formats.

---

1-8: Well-organized test file for SLURM-independent parsing!

The module docstring clearly states the purpose, and the test organization supports the PR objective of allowing the plugin to be imported without SLURM installed.

---

10-36: No issue found with logger=None handling for this test.

The test data includes populated comment fields (rule names in the third position) and the CPU efficiency calculation will never yield values below 0%, so the logger.warning() calls at lines 144 and 216-229 in parse_sacct_data are never triggered for this specific test case. The test will pass successfully with logger=None.

Likely an incorrect or invalid review comment.