Generalise submission and cancellation arguments

[AlecThomson]

Closes #640

[jacobtomlinson]

This looks fine to me! I do wonder if we should expose cancel_command as a keyword argument on the SLURMCluster and pass that down to SLURMJob, in case users want to override it with something else.

[AlecThomson]

I found that the HTCondor class already had something similar. I've added this to the base Job class in core. The downside is that this adds some extra boilerplate.

I've reworked the HTCondorJob and SLURMJob to make use of the new functionality

[guillaumeeb]

Why is all this code both in the Core class and the class inheriting it?

[guillaumeeb]

Also, the test failures in slurm (and maybe others) looks related to this change.

[jacobtomlinson]

CI is now mostly happy on main so I've just merged in so we can see up to date CI failures.

[AlecThomson]

Sorry for the long lead time on this, everyone. I got myself tripped up between the methods on the base and inheriting classes - as noted by @guillaumeeb's comment. I believe I've got this all sorted now. Hopefully the CI will catch any lingering issues.

[jacobtomlinson]

Thanks @AlecThomson! Looks like there are some linting issues (make sure you run pre-commit install) and some slurm issues.

[AlecThomson]

Hmm - looks like some kind of timing error on the test. I don't quite understand why it's failing... 🤔

>                   assert time() < start + QUEUE_WAIT
E                   assert 1723021595.3378592 < (1723021535.2718754 + 60)
E                    +  where 1723021595.3378592 = time()

https://github.com/dask/dask-jobqueue/actions/runs/10278490598/job/28450471395#step:7:425

[jacobtomlinson]

It is calling cluster.scale(n) and then waiting for the cluster to scale. The time assertion is just a timeout, so it's not scaling to the correct number in the time allowed.

Note: We don't use client.wait_for_workers(n) because that checks for "at least n workers" so doesn't wait when scaling down (xref dask/distributed#6374).