Skip to content

fix job outcome reporting on cancelled job #22

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
Apr 29, 2024

Conversation

alexrashed
Copy link
Member

Motivation

During the testing of the new outcome feature introduced in #21, we realized that the reporting for jobs does not work correctly in case of cancellations (due to timeouts or manual cancellations).
This PR addresses this issue.

Changes

  • Properly handle cancelled job outcomes.

Testing

  • Added some new jest tests.

@alexrashed alexrashed added the bug Something isn't working label Apr 26, 2024
@alexrashed alexrashed requested a review from silv-io April 26, 2024 11:25
@alexrashed alexrashed self-assigned this Apr 26, 2024
@alexrashed alexrashed force-pushed the fix-job-reporting-on-cancellation branch from 8bd2509 to 558ac22 Compare April 26, 2024 11:36
Copy link
Member

@silv-io silv-io left a comment

Choose a reason for hiding this comment

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

LGTM!

I think the original reason why we didn't add that was that forcing a re-run would cause the old run to cancel. So merging a PR right after another could show up as a failed workflow run.

However, thinking more about it this might have been a erroneous thought, since cancelling the workflow would then in turn also cancel the tinybird reporting itself.

@alexrashed
Copy link
Member Author

Actually, that's only true if you have only one concurrent run (which usually isn't the case on main branches. But I think a cancelled run would still run these actions, since in our suggested config we use always.

Anyways, I think this is the right change right now :)

@alexrashed alexrashed merged commit 518790f into main Apr 29, 2024
@alexrashed alexrashed deleted the fix-job-reporting-on-cancellation branch April 29, 2024 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants