feat: multicluster - Githubissues

snakemake / snakemake-executor-plugin-slurm

A Snakemake executor plugin for submitting jobs to a SLURM cluster

MIT License

18 stars 19 forks source link

feat: multicluster #56

Closed cmeesters closed 3 months ago

cmeesters commented 7 months ago

Putative fix for issue #53

meliamne commented 7 months ago

when specifying the cluster in sbatch the output changes to Submitted batch job 61317320 on cluster wice. This causes line 162 to mistakenly pick up the name of the cluster instead of the jobid.

https://github.com/snakemake/snakemake-executor-plugin-slurm/blob/7103c7eb1d48ecae568e1b9a92b79c9cb6b226d5/snakemake_executor_plugin_slurm/__init__.py#L162

cmeesters commented 7 months ago

@meliamne Thank you, your suggestion went into my last commit.

cmeesters commented 7 months ago

@johanneskoester does the current state find your approval?

I dropped

self.__clusters as an attribute, instead
sacct and scancel use --clusters=all, as suggested.
in addition, please take a look at the regex use in line 159 .

cmeesters commented 6 months ago

@johanneskoester we do not have a test, yet (due to faulty multicluster setup in the test-engine). Should we merge regardless, considering that the test will take considerable time and effort to implement?

cmeesters commented 5 months ago

@johanneskoester not sure, isn't the requested change already in there, resp. the issue solved?

cmeesters commented 4 months ago

@johanneskoester please have a look, after all this rebasing and fixing, I am a little puzzled. It's embarrassing, but it took too long.

cmeesters commented 3 months ago

We (@johanneskoester ) and me discussed that we do not have a solution for a multicluster test at hand for the time being. Hence, this PR is merged without a dedicated test.

I am sorry, I could merge this sooner. Too many things kept coming up, then holidays ....

nigiord commented 3 months ago

Thanks for this merge! I think there is a mistake in the documentation though:

https://github.com/snakemake/snakemake-executor-plugin-slurm/blob/03d474f74f703c4e4530bbeda8f16dcf43764263/docs/further.md?plain=1#L122

The snakemake resources seems to be clusters (with an s), and not cluster:

https://github.com/snakemake/snakemake-executor-plugin-slurm/blob/03d474f74f703c4e4530bbeda8f16dcf43764263/snakemake_executor_plugin_slurm/__init__.py#L140

I also made a comment regarding the way jobid is now parsed: https://github.com/snakemake/snakemake-executor-plugin-slurm/issues/121#issuecomment-2270905243

Cheers,

cmeesters commented 3 months ago

Ah, thanks. Will fix this asap (= not today).