batch system -> job runner #3992

MetRonnie · 2020-12-11T18:42:10Z

These changes close #3910. Yikes.

Global config change:

[platforms][X]batch system -> [platforms][X]job runner
[platforms][X]batch submit command template -> [platforms][X]job runner command template

Plus: Change all permutations of batch system, batch_sys etc to an equivalent job runner, apart from where batch system refers to the workflow config item [runtime][X][job]batch system.

I've tried to batch the changes into somewhat digestible commits for reviewing.

Requirements check-list

I have read CONTRIBUTING.md and added my name as a Code Contributor.
Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).

Appropriate tests are included (functional).

Appropriate change log entry included. (in job runner: mop up #4004)

(master branch) I have opened a documentation PR at batch sys -> job runner cylc-doc#180.

No dependency changes.

cylc/flow/batch_sys_handlers/loadleveler.py

MetRonnie · 2020-12-11T22:42:26Z

cylc/flow/data_messages.proto

-    optional string batch_sys_job_id = 9;
-    optional string batch_sys_name = 10;
+    optional string job_id = 9;
+    optional string job_runner_name = 10;


@dwsutherland Can I get a quick check from you that the GraphQL and protobuf stuff has been done correctly? (this commit b64822c and this one 7a23e2f#diff-3c027c5232fb05de824ca58eb160f34cd3e253500824026046cbf56fc526431a)

Looks good to me, as long as the instructions at the top were followed, the binary used to generate the module is the same version protobuf, and tests pass 👍

The diff in the generated module looks appropriate.. Will re-generate on review and see if there are any changes 👍

and batch submit command template -> job runner command template

Deprecate batch_sys event handler templates %(batch_sys_job_id)s -> %(job_id)s %(batch_sys_name)s -> %(job_runner_name)s

Add test for %(batch_sys_job_id)s -> %(job_id)s and %(batch_sys_name)s -> %(job_runner_name)s

The warning about using deprecated "%(batch_sys_job_id)s" etc in event handlers now occurs on cylc validate

oliver-sanders · 2020-12-14T13:47:31Z

cylc/flow/job_runner_handlers/__init__.py

-   :toctree: batch-sys-handlers
-   :template: automodule_batch_sys_handlers.rst
+   :toctree: job-runner-handlers
+   :template: automodule_job_runner_handlers.rst


Note, this is the file :template: is pointing at - https://github.com/cylc/cylc-doc/blob/master/src/_templates/automodule_batch_sys_handlers.rst

oliver-sanders · 2020-12-14T15:48:26Z

cylc/flow/task_events_mgr.py

@@ -1125,22 +1127,27 @@ def _setup_custom_event_handlers(self, itask, event, message):
                        quote(str(self.uuid_str)),
                    EventData.TryNum.value:
                        itask.get_try_num(),
+                    # next 2 are deprecated - still here for back compat:


Lesson learned from Cylc 5/6.

Back compat comments make sense at the time, but further down the road no-one can remember why they were added, when the change was made or when the support should be removed.

Suggest marking comments with BACK COMPAT so they can be grepped for, there doesn't seem to be a standard for this so we get to invent one.

Suggested change

# next 2 are deprecated - still here for back compat:

# BACK COMPAT

# For: Cylc <= 7

# Remove at: Cylc9 - pending announcement of upcoming deprecation

# https://github.com/cylc/cylc-flow/pull/3992

# next 2 (JobID_old, JobRunnerName_old) are deprecated

tests/functional/directives/README

oliver-sanders · 2020-12-14T16:08:25Z

Looks good, very thorough!

hjoliver · 2020-12-14T21:48:43Z

The failed test tests/f/cylc-poll/04-poll-multi-hosts.t was fixed by #3995 (so a rebase will get it).

Plus tidy & remove remaining references to obsoleted [test battery] config section

tests/functional/job-file-trap/00-sigusr1.t

oliver-sanders · 2020-12-15T16:42:39Z

~~One small hitch with Cylc env vars, after that should be good to go.~~

hjoliver

Looks good 👍

MetRonnie added the config change Involves a change to global or workflow config label Dec 11, 2020

MetRonnie added this to the cylc-8.0.0 milestone Dec 11, 2020

MetRonnie requested review from hjoliver and oliver-sanders December 11, 2020 18:42

MetRonnie self-assigned this Dec 11, 2020

MetRonnie commented Dec 11, 2020

View reviewed changes

cylc/flow/batch_sys_handlers/loadleveler.py Show resolved Hide resolved

MetRonnie commented Dec 11, 2020

View reviewed changes

MetRonnie force-pushed the batch-sys branch from a70d146 to 2ba018e Compare December 11, 2020 23:04

hjoliver mentioned this pull request Dec 13, 2020

Remove unused code in cylc.flow.config (template keys) #3993

Closed

7 tasks

MetRonnie added 17 commits December 14, 2020 09:52

Global conf: platforms: batch system -> job runner

a71e579

and batch submit command template -> job runner command template

Tests: update global platforms conf: batch system -> job runner

abfed71

Wider rename of batch system -> job runner

ac57c47

Wider rename of batch system -> job runner, pt 2

879af61

Tidy

9d18bb5

Wider rename of batch system -> job runner, pt 3

accf98a

Deprecate batch_sys event handler templates %(batch_sys_job_id)s -> %(job_id)s %(batch_sys_name)s -> %(job_runner_name)s

Warn about deprecated event handler template vars

0ccbf39

Add test for %(batch_sys_job_id)s -> %(job_id)s and %(batch_sys_name)s -> %(job_runner_name)s

Wider rename of batch system -> job runner, pt 4

d619617

Wider rename of batch system -> job runner, pt 5

aae9bf6

Move deprecation warning to cfgspec.suite

f36ab41

The warning about using deprecated "%(batch_sys_job_id)s" etc in event handlers now occurs on cylc validate

Rename BATCH_SYS_HANDLER -> JOB_RUNNER_HANDLER

8ba951b

Rename batch_sys_manager -> job_runner_mgr

53fd9b0

Rename BatchSysManager -> JobRunnerManager

efc0376

Rename batch_sys_mgr -> job_runner_mgr

f8ca4f1

Rename batch_sys_handlers -> job_runner_handlers

908c9e4

Finish wider rename of batch system -> job runner

5612545

Fix documentation in globalcfg

92847e5

MetRonnie force-pushed the batch-sys branch from 2ba018e to 92847e5 Compare December 14, 2020 10:12

oliver-sanders mentioned this pull request Dec 14, 2020

job runners: upgrade doc examples to platforms #3997

Closed

oliver-sanders reviewed Dec 14, 2020

View reviewed changes

Apply suggestion from code review

5ec6063

Plus tidy & remove remaining references to obsoleted [test battery] config section

oliver-sanders reviewed Dec 15, 2020

View reviewed changes

tests/functional/job-file-trap/00-sigusr1.t Show resolved Hide resolved

hjoliver approved these changes Dec 17, 2020

View reviewed changes

oliver-sanders approved these changes Dec 17, 2020

View reviewed changes

oliver-sanders merged commit f38b47c into cylc:master Dec 17, 2020

oliver-sanders mentioned this pull request Dec 17, 2020

BACK COMPAT: remove old compat and update comments #4001

Merged

7 tasks

MetRonnie deleted the batch-sys branch December 17, 2020 11:50

MetRonnie modified the milestones: cylc-8.0.0, cylc-8.0a3 Dec 17, 2020

MetRonnie mentioned this pull request Dec 17, 2020

Fix reference to batch sys -> job runner #4003

Merged

oliver-sanders mentioned this pull request Dec 17, 2020

job runner: mop up #4004

Merged

7 tasks

MetRonnie mentioned this pull request Dec 17, 2020

batch sys -> job runner cylc/cylc-doc#180

Merged

hjoliver modified the milestones: cylc-8.0a3, cylc-8.0b0 Feb 25, 2021

MetRonnie mentioned this pull request Apr 27, 2021

conf: ensure all proposed changes have been enacted #3696

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

batch system -> job runner #3992

batch system -> job runner #3992

MetRonnie commented Dec 11, 2020 •

edited

Loading

MetRonnie Dec 11, 2020 •

edited

Loading

dwsutherland Dec 14, 2020

dwsutherland Dec 14, 2020

oliver-sanders Dec 14, 2020

oliver-sanders Dec 14, 2020

oliver-sanders commented Dec 14, 2020

hjoliver commented Dec 14, 2020

oliver-sanders commented Dec 15, 2020 •

edited

Loading

hjoliver left a comment

-                    # next 2 are deprecated - still here for back compat:
+                    # BACK COMPAT
+                    # For: Cylc <= 7
+                    # Remove at: Cylc9 - pending announcement of upcoming deprecation
+                    # https://github.com/cylc/cylc-flow/pull/3992
+                    # next 2 (JobID_old, JobRunnerName_old) are deprecated

batch system -> job runner #3992

batch system -> job runner #3992

Conversation

MetRonnie commented Dec 11, 2020 • edited Loading

MetRonnie Dec 11, 2020 • edited Loading

Choose a reason for hiding this comment

dwsutherland Dec 14, 2020

Choose a reason for hiding this comment

dwsutherland Dec 14, 2020

Choose a reason for hiding this comment

oliver-sanders Dec 14, 2020

Choose a reason for hiding this comment

oliver-sanders Dec 14, 2020

Choose a reason for hiding this comment

oliver-sanders commented Dec 14, 2020

hjoliver commented Dec 14, 2020

oliver-sanders commented Dec 15, 2020 • edited Loading

hjoliver left a comment

Choose a reason for hiding this comment

MetRonnie commented Dec 11, 2020 •

edited

Loading

MetRonnie Dec 11, 2020 •

edited

Loading

oliver-sanders commented Dec 15, 2020 •

edited

Loading