cylc-flow icon indicating copy to clipboard operation
cylc-flow copied to clipboard

Lint.job runner specific timeout

Open wxtim opened this issue 1 year ago • 4 comments

Closes #6131

Check List

  • [x] I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • [x] Contains logically grouped changes (else tidy your branch by rebase).
  • [x] Does not contain off-topic changes (use other PRs for other changes).
  • [x] Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • [x] Tests are included (or explain why tests are not needed).
  • [x] CHANGES.md entry included if this is a change that can affect users
  • [x] Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • [x] If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

wxtim avatar Jun 13 '24 15:06 wxtim

Marked my review comment as resolved but then it's hidden all the comments on the files, which are not resolved. Now I can't change the review comment to unresolved 😑

Please unhide the review and look at the comments

MetRonnie avatar Jun 20 '24 11:06 MetRonnie

Also should remove any instances of time limit directives from the docs? E.g. https://github.com/cylc/cylc-flow/blob/86d24c3c1e63806590a89e75cc4202851feef1af/cylc/flow/job_runner_handlers/sge.py#L40

MetRonnie avatar Jun 20 '24 11:06 MetRonnie

Also should remove any instances of time limit directives from the docs? E.g.

https://github.com/cylc/cylc-flow/blob/86d24c3c1e63806590a89e75cc4202851feef1af/cylc/flow/job_runner_handlers/sge.py#L40

This appears to only be an issue for SGE where a not below tells you not to do precisely this!

n.b. Some of these directives still appear in documentation where examples of the job file show how the execution time limit will be converted to directives.

wxtim avatar Jun 20 '24 12:06 wxtim

The only thing it doesn't pick up on is when multiple directives are specified on the same setting, e.g.

[[[directives]]]
    -l = walltime=60:nodes=1

But otherwise I have tested and am happy with this

Not a pattern we see too often

wxtim avatar Jun 21 '24 12:06 wxtim