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

docs: Clarification of --cluster-stats docs & elaborating on the situation where job ids are not passed to the status script #1221

Merged

Conversation

iamh2o
Copy link
Contributor

@iamh2o iamh2o commented Oct 16, 2021

Description

This PR is a small clarification and elaboration on how to get --cluster-status to work .

  1. The current docs were not clear to me about how to get this feature to work until I read them several times and had numerous false starts. So, I tried to spell out some of the information which was implied regarding how this feature works that I believe will make this more approachable to a wider audience.

  2. Described a frequently faced challenge that must be solved for this feature to work as stated (well, must be solved by some number of users based on my hunting for answers prior to writing this PR. Specifically, enumerating a few workarounds for when the cluster submission script returns a string with more than the job id to snakemake, and snakemake then passes that entire string to the cluster-status script, causing it to best case crash the script, worst case the string contains characters which cause the shell to not even execute the script. The current docs, as written, imply snakemake has the job id and no thought need to be give to this aspect of the feature.

My intention was to clarify how this feature operated and make brief suggestions for solutions to the string, not job id, conundrum. Which, I do not consider a snakemake bug necessarily, it seems reasonable to ask users to parse this for themselves, they just should be aware it might need to be done.

QC

I added only a handful of sentences, which I proofread.

  • [ a doc clarification only, no s/w changes ] The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • [ docs updated ] The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

…a common challenge not called out in the docs. Specically, suggestions for workarounds when the cluster submission script returns more than the job id to snakemake, and snakemake passes that entire string to the cluster-status script.... sometimes the string includes characters that prevent the script from even executing. I hope this clarification makes the feature more approachable and useful :-)
@iamh2o iamh2o requested a review from johanneskoester as a code owner Oct 16, 2021
@iamh2o iamh2o changed the title Clarification of --cluster-stats docs & elaborating on the situation where job ids are not passed to the status script docs: Clarification of --cluster-stats docs & elaborating on the situation where job ids are not passed to the status script Oct 16, 2021
@jdblischak
Copy link
Contributor

jdblischak commented Oct 20, 2021

@iamh2o I like your documentation update. I wish it had been available when I was trying to understand the --cluster-status flag.

worst case the string contains characters which cause the shell to not even execute the script

I ran into this problem with Slurm. In a multi-cluster setup, Slurm returns the name of the cluster and the job ID separated by a semi-colon (when using --parsable); which breaks everything.

I submitted PR #977 to specifically address this problem. By quoting the string passed to the cluster status script as a single string, this allows your script to properly parse it. I'd appreciate any feedback on my PR.

@sonarcloud
Copy link

sonarcloud bot commented Oct 21, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johanneskoester johanneskoester merged commit ed0e4a2 into snakemake:main Oct 21, 2021
6 checks passed
pvandyken pushed a commit to pvandyken/snakemake that referenced this pull request Nov 15, 2021
…ation where job ids are not passed to the status script (snakemake#1221)

* clarification on how to get --cluster-status to work. And describing a common challenge not called out in the docs. Specically, suggestions for workarounds when the cluster submission script returns more than the job id to snakemake, and snakemake passes that entire string to the cluster-status script.... sometimes the string includes characters that prevent the script from even executing.  I hope this clarification makes the feature more approachable and useful :-)

* Update additional_features.rst

Co-authored-by: Johannes Köster <johannes.koester@uni-due.de>
@iamh2o iamh2o deleted the cluster_status_docs_clarification branch Nov 29, 2021
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.

None yet

3 participants