-
Notifications
You must be signed in to change notification settings - Fork 515
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
fix: addressing #2197 by allowing 256 character account names in slurm #2198
fix: addressing #2197 by allowing 256 character account names in slurm #2198
Conversation
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a trivial change to me, where I can't see any negative side effects. So if this fixes the problem locally, this should be fine to merge.
Also, we should follow up on your suggestion for a more thorough fix: But this fix works for now, and doesn't need any further digging into the slurm docs... 😅 |
Would greatly appreciate this change. Most of TU Delft's accounts are longer than 20 characters (examples), meaning we cannot make use of Edit: Can confirm that this fix works for me. |
Sorry for the delay. This slipped my attention. If something like that happens, always feel free to ping me via discord (username johanneskoester). |
I'll release this today. |
No worries! Thank you 🙌 |
…s in slurm (snakemake#2198) ### Description This changes the format string on the calls to 'sacct' and 'sacctmgr' to allow for long slurm account names. This fix seems to work on our slurm cluster. ### QC <!-- Make sure that you can tick the boxes below. --> * [ ] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] 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).
🤖 I have created a release *beep* *boop* --- ## [7.29.0](v7.28.3...v7.29.0) (2023-06-21) ### Features * introduce --workflow-profile for additional workflow specific profiles that overwrite global profiles; add ability to define key-value CLI flags like --set-threads or --set-resources as multi-level dictionaries in profile config yaml files ([#2310](#2310)) ([9675c17](9675c17)) ### Bug Fixes * addressing [#2197](#2197) by allowing 256 character account names in slurm ([#2198](#2198)) ([ab58c65](ab58c65)) * removed distutils from snakemake ([#2312](#2312)) ([9b8c362](9b8c362)) * Update __init__.py to move "file" param to "print" ([#2291](#2291)) ([92352b6](92352b6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Description
This changes the format string on the calls to 'sacct' and 'sacctmgr' to allow for long slurm account names. This fix seems to work on our slurm cluster.
QC
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).