Skip to content

[feat] Make job completion time available as a logging variable #1090

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 17 commits into from
Dec 18, 2019
Merged

[feat] Make job completion time available as a logging variable #1090

merged 17 commits into from
Dec 18, 2019

Conversation

@ekouts
Copy link
Contributor

@ekouts ekouts commented Dec 6, 2019 •
edited
Loading

The attribute is more accurate for the slurm scheduler that uses sacct (but not the squeue job scheduler). The accuracy for the rest is dependent on the polling rate of the running tasks.

Closes #1044.

@pep8speaks
Copy link

pep8speaks commented Dec 6, 2019 •
edited
Loading

Hello @ekouts, Thank you for updating!

Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide

Comment last updated at 2019-12-18 18:48:57 UTC

@ekouts ekouts requested a review from vkarak December 6, 2019 12:36
@ekouts ekouts changed the title Add check_completion_time attribute to perflogs [feat] Add check_completion_time attribute to perflogs Dec 6, 2019
@ekouts ekouts self-assigned this Dec 6, 2019
@ekouts ekouts added this to the ReFrame sprint 2019w48 milestone Dec 6, 2019
@ekouts
Copy link
Contributor Author

ekouts commented Dec 6, 2019

@jenkins-cscs retry kesch

vkarak
vkarak requested changes Dec 10, 2019
View reviewed changes
@vkarak vkarak changed the title [feat] Add check_completion_time attribute to perflogs [feat] Make job completion time available as a logging variable Dec 10, 2019
@ekouts
Copy link
Contributor Author

ekouts commented Dec 11, 2019

@jenkins-cscs retry kesch

vkarak
vkarak requested changes Dec 11, 2019
View reviewed changes
@codecov-io
Copy link

codecov-io commented Dec 12, 2019 •
edited
Loading

Codecov Report

Merging #1090 into master will decrease coverage by 0.15%.
The diff coverage is 60.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1090      +/-   ##
==========================================
- Coverage    91.7%   91.55%   -0.16%     
==========================================
  Files          81       81              
  Lines       11121    11172      +51     
==========================================
+ Hits        10199    10228      +29     
- Misses        922      944      +22
Impacted Files Coverage Δ
reframe/core/logging.py 83.74% <100%> (+0.2%) ⬆️
reframe/core/schedulers/local.py 100% <100%> (ø) ⬆️
reframe/utility/os_ext.py 89.75% <100%> (+0.06%) ⬆️
reframe/core/schedulers/slurm.py 53.96% <31.25%> (-1.04%) ⬇️
unittests/test_environments.py 72.07% <40%> (-1.08%) ⬇️
reframe/core/environments.py 88.28% <40%> (-4.1%) ⬇️
unittests/test_launchers.py 92.72% <50%> (-0.8%) ⬇️
reframe/core/schedulers/pbs.py 67.5% <50%> (-0.45%) ⬇️
reframe/core/schedulers/__init__.py 95.3% <91.66%> (-0.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 574bee6...7a359d6. Read the comment docs.

vkarak
vkarak requested changes Dec 14, 2019
View reviewed changes
@ekouts
Address PR comments
f84b7c9 
- Change the environment context manager so that it uses Environment class
- Add an option for logging in run_command
- Set completion_time of slurm scheduler inside the respective function instead of update_state
vkarak
vkarak requested changes Dec 16, 2019
View reviewed changes
vkarak
vkarak requested changes Dec 17, 2019
View reviewed changes
@vkarak
Copy link
Contributor

vkarak commented Dec 18, 2019

Code-wise this PR is ready. Only remaining thing is to document the new logging variable.

@vkarak
Copy link
Contributor

vkarak commented Dec 18, 2019

I changed the name of the logging variable to check_job_completion_time, since it is more precise.

@vkarak
Copy link
Contributor

vkarak commented Dec 18, 2019

@brandongc I will merge this one. You should now be able to use %(check_job_completion_time)s inside the log format to get the actual completion time of the job. The timestamp will be formatted based on the value of datefmt.

@vkarak vkarak merged commit 3cc72fc into reframe-hpc:master Dec 18, 2019
@ekouts ekouts deleted the feature/check_completion_time branch January 16, 2020 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vkarak vkarak vkarak approved these changes

Assignees

@ekouts ekouts

Labels

prio: normal request for enhancement

Projects

None yet

Milestone

ReFrame sprint 2019w50

Development

Successfully merging this pull request may close these issues.

perflogging timestamps with async policy

4 participants

@ekouts @pep8speaks @codecov-io @vkarak