Skip to content
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

job exit script #2868

Merged
merged 6 commits into from
Nov 21, 2018
Merged

job exit script #2868

merged 6 commits into from
Nov 21, 2018

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Nov 19, 2018

(Coded on a long-haul flight, when I couldn't sleep).

Add support for a new exit-script runtime setting, for scripting that is executed at the very last before a job exits , whether normal exit, or fail, or kill (except SIGKILL, obviously) [successfully].

Similar to the existing err-script, which executes on job failure.

Use case example: automatic deletion heavily used job temp directories, e.g. for jobs that don't use the work directories provided by cylc. (This is evidently a thing for direct ports from SMS at one of our main sites).

Tests added (expect a few test failures based on a new config item being added without adjusting the config tests - TODO)

I won't bother adding documentation until or if agreed that this change is good (or at least, not unreasonable). [documented]

One thing I'm not sure of: perhaps the exit script should only run on successful exit? Or do we need both behaviours as an option? [success only]

Note "cylc kill" for bg jobs use SIGKILL, so the exit-script doesn't
run in that case.
@hjoliver hjoliver added this to the soon milestone Nov 19, 2018
@hjoliver hjoliver self-assigned this Nov 19, 2018
@hjoliver
Copy link
Member Author

(I suppose it would be sensible to have this run only for successful exit, along side the err script for job fail).

Copy link
Contributor

@matthewrmshin matthewrmshin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment. Looks OK otherwise. Note an old config test needs adding a bunch of new exit_script = to pass.

tests/jobscript/19-exit-script.t Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member Author

Old config test fixed.

@trwhitcomb
Copy link
Collaborator

One area this is useful (for me at least) is in the handling of the extra log files entry. Right now, those log files just reside in the job log directory. This is great, as it gives me a way to view extra log files through the GUI/CLI - however, the way things are currently set up, I need log files in the share directory, not in a job-specific log directory. I had thought about adding a post-script that copied/linked the log files to the approrpriate locations for GUI viewing later, but ran into a challenge that it wouldn't work if the job wasn't successful. This might help with some of that.

@hjoliver hjoliver mentioned this pull request Nov 20, 2018
3 tasks
@hjoliver
Copy link
Member Author

Changing this to execute exit-script on job success only.

@hjoliver hjoliver requested a review from kinow November 20, 2018 03:20
@hjoliver
Copy link
Member Author

Now run on job success only; documented; ready for review.

Note I've left my use of eval above - but feel free to convince me to replace it with more complex code 😁

@hjoliver hjoliver modified the milestones: soon, next release Nov 20, 2018
@hjoliver
Copy link
Member Author

Assigned to next-release as this is a pretty simple change (and needed at the aforementioned site).

Copy link
Contributor

@matthewrmshin matthewrmshin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks OK. Feel free to agree/ignore my eval comment.

Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, docs too. Works as expected on my environment (and Today-I-Learned --set=VAR=VALUE 🌟 ).

# after: cylc run --no-detach --debug --verbose --set=EXIT=true exit-script

kinow@ranma:~/cylc-run/exit-script$ cat log/job/1/foo/01/job.out 
Suite    : exit-script
Task Job : 1/foo/01 (try 1)
User@Host: kinow@ranma

HELLO
2018-11-21T22:57:31+13:00 INFO - started
kinow@ranma:~/cylc-run/exit-script$ cat log/job/1/foo/01/job.err 
Sending DEBUG MODE xtrace to job.xtrace
2018-11-21T22:57:32+13:00 DEBUG - Loading site/user global config files
2018-11-21T22:57:32+13:00 DEBUG - Reading file /home/kinow/Development/python/workspace/cylc/etc/global.rc
2018-11-21T22:57:32+13:00 DEBUG - deprecated items were automatically upgraded in 'site config':
2018-11-21T22:57:32+13:00 DEBUG -  * (6.11.0) [state dump rolling archive length] - DELETED (OBSOLETE)
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [communication][base port] -> [suite servers][run ports] - Format as range list
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [communication][maximum number of ports] - DELETED (OBSOLETE)
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [suite host scanning] -> [suite servers] - value unchanged
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [suite servers][hosts] -> [suite servers][scan hosts] - value unchanged
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [suite logging][roll over at start-up] - DELETED (OBSOLETE)
2018-11-21T22:57:32+13:00 DEBUG - Reading file /home/kinow/.cylc/global.rc
2018-11-21T22:57:32+13:00 CRITICAL - failed/EXIT
2018-11-21T22:57:32+13:00 DEBUG - Loading site/user global config files
2018-11-21T22:57:32+13:00 DEBUG - Reading file /home/kinow/Development/python/workspace/cylc/etc/global.rc
2018-11-21T22:57:32+13:00 DEBUG - deprecated items were automatically upgraded in 'site config':
2018-11-21T22:57:32+13:00 DEBUG -  * (6.11.0) [state dump rolling archive length] - DELETED (OBSOLETE)
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [communication][base port] -> [suite servers][run ports] - Format as range list
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [communication][maximum number of ports] - DELETED (OBSOLETE)
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [suite host scanning] -> [suite servers] - value unchanged
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [suite servers][hosts] -> [suite servers][scan hosts] - value unchanged
2018-11-21T22:57:32+13:00 DEBUG -  * (7.8.0) [suite logging][roll over at start-up] - DELETED (OBSOLETE)
2018-11-21T22:57:32+13:00 DEBUG - Reading file /home/kinow/.cylc/global.rc
EXIT Oops!

@kinow kinow merged commit 0e48db7 into cylc:master Nov 21, 2018
@kinow
Copy link
Member

kinow commented Nov 21, 2018

Two approvals, and set to next-release. LGTM.

@hjoliver hjoliver deleted the exit-script branch November 21, 2018 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants