Skip to content

Adjust total time for failed jobs #346

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

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Conversation

emilylin0
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.
@emilylin0 emilylin0 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.

@emilylin0
Copy link
Contributor Author

emilylin0 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

@emilylin0 emilylin0 merged commit b4e8744 into main Feb 22, 2023
@emilylin0 emilylin0 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