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

Adjust total time for failed jobs #346

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Conversation

emily827
Copy link
Contributor

This PR ensures that total_time is always going to be updated, even when a job fails.

Previously, if a job failed, total_time would not include the run-time of the iteration that threw the exception.

Motivation
We want to create a metric to keep track of a job's total run-time. This metric is going to be reported whenever a job completes successfully, fails, or is interrupted

This commit ensures that total_time is updated even when a job
has failed. Previously, total_time would not include the run-time
of the iteration that failed.
@emily827 emily827 force-pushed the el/always_adjust_total_time branch from 39fb7c8 to d93275a Compare February 22, 2023 16:49
Copy link

@solackerman solackerman left a comment

Choose a reason for hiding this comment

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

makes sense 👍 it looks like some tests are failing though.

@emily827
Copy link
Contributor Author

emily827 commented Feb 22, 2023

makes sense 👍 it looks like some tests are failing though.

I think that's because the main branch is failing 🤔 Those failures don't look related to my changes I think - #339 and steveklabnik/mono_logger#12

@emily827 emily827 merged commit b4e8744 into main Feb 22, 2023
@emily827 emily827 deleted the el/always_adjust_total_time branch February 22, 2023 19:17
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems February 26, 2023 14:32 Inactive
@lavoiesl lavoiesl mentioned this pull request Aug 15, 2023
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.

2 participants