Skip to content

Conversation

@cmeesters
Copy link
Member

@cmeesters cmeesters commented Jan 21, 2026

The substring "%i|%T" is now quoted with shlex.quote. This ensures that it will work on any standard shell and should fix issue #395.

Summary by CodeRabbit

Bug Fixes

  • Enhanced stability of job status queries by properly handling shell command arguments.

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

@cmeesters cmeesters requested a review from cademirch January 21, 2026 16:49
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

Walkthrough

Added shell command argument quoting using shlex.quote() to safely pass the format string "%i|%T" to the squeue command in query_job_status_squeue(), preventing unintended shell interpretation of special characters.

Changes

Cohort / File(s) Summary
Shell argument quoting
snakemake_executor_plugin_slurm/job_status_query.py
Introduced `shlex.quote("%i

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • johanneskoester

Poem

🐰 A format string once ran wild and free,
With pipes and percents that shouldn't be,
But shlex.quote came hopping by,
To shield the shell with a careful sigh,
Now safely quoted, the job status flies!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing a shell quoting issue by using shlex to quote a piped substring format argument.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

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/job_status_query.py (1)

146-155: Quoting is stripped by shlex.split/join, so | is still interpreted by the shell.

format_arg is quoted, but query_command = " ".join(shlex.split(...)) removes those quotes, producing --format=%i|%T again. If this string is executed with shell=True, the pipe still acts as a pipeline and the fix won’t work.

✅ Proposed fix (preserve quoting during normalization)
-    query_command = " ".join(shlex.split(query_command))
+    query_command = " ".join(
+        shlex.quote(arg) for arg in shlex.split(query_command)
+    )

@cmeesters cmeesters merged commit 7aa7fc3 into main Jan 23, 2026
6 checks passed
@cmeesters cmeesters deleted the fix/squeue_quoting_issue branch January 23, 2026 11:24
cmeesters pushed a commit that referenced this pull request Feb 9, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.2.0](v2.1.0...v2.2.0)
(2026-02-09)


### Features

* job name prefix
([#408](#408))
([5fa0d33](5fa0d33))


### Bug Fixes

* cancel on multicluster
([#401](#401))
([cb6124b](cb6124b))
* decreasing job query verbosity
([#405](#405))
([6649881](6649881))
* extracting job id from convoluted output, if necessary
([#375](#375))
([950c909](950c909))
* quoting piped substring with shlex
([#402](#402))
([7aa7fc3](7aa7fc3))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

* **New Features**
  * Added job name prefix capability.

* **Bug Fixes**
  * Fixed cancel operation on multicluster environments.
  * Improved job query verbosity output.
  * Fixed job ID extraction from complex output formats.
  * Fixed substring quoting in piped commands.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants