Cancel running jobs by event rather than previous commit SHA#3076
Cancel running jobs by event rather than previous commit SHA#3076nforro wants to merge 2 commits intopackit:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a cancel_cutoff_time mechanism to prevent accidental cancellation of newly triggered jobs within the same event batch. This involves adding cancel_cutoff_time to Event and EventData classes, and modifying get_running methods across various build and test models (Copr, Koji, Testing Farm, Log Detective) to filter jobs based on this new timestamp and project_event_type/event_id instead of commit_sha. A new get_latest_datetime_for_event method was added to PipelineModel to determine this cutoff time. Review comments highlight the need for robust checks to ensure cancel_cutoff_time and db_project_event are not None to prevent unintended job cancellations and to improve docstring clarity for the created_before parameter in get_running methods.
|
✔️ pre-commit SUCCESS in 1m 56s |
|
/gemini review |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 44s |
There was a problem hiding this comment.
Code Review
This pull request refactors the job cancellation logic to use event-based filtering with a cancel_cutoff_time instead of relying solely on commit_sha. This change introduces a cancel_cutoff_time field to Event and EventData classes, which is then used to query running Copr builds, Koji builds, Testing Farm runs, and Log Detective runs. A new PipelineModel.get_latest_datetime_for_event method is added to retrieve the most recent pipeline datetime for a given project event, which helps determine the cancel_cutoff_time. This ensures that only jobs related to a specific event and initiated before a certain time are considered for cancellation, preventing unintended cancellations of unrelated jobs. Numerous tests have been updated to reflect these changes, including mocking the new cancel_cutoff_time and event-based parameters, and new tests have been added to verify the correctness of the new cancellation logic.
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 51s |
Some triggers such as comment commands have no relation to a commit SHA, so canceling doesn't work for them. We can use project event instead to identify running jobs that should be canceled, but event IDs are reused and we need to exclude current jobs, i.e. those that have been started by our trigger. To make that possible, before starting any tasks for a job record datetime of the most recent pipeline of the triggering event and use that as a cutoff time when filtering running jobs to cancel. Signed-off-by: Nikola Forró <nforro@redhat.com>
Signed-off-by: Nikola Forró <nforro@redhat.com> Assisted-by: Claude Opus 4.6 via Claude Code
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 57s |
3 participants
Some triggers such as comment commands have no relation to a commit SHA, so canceling doesn't work for them. We can use project event instead to identify running jobs that should be canceled, but event IDs are reused and we need to exclude current jobs, i.e. those that have been started by our trigger. To make that possible, before starting any tasks for a job record datetime of the most recent pipeline of the triggering event and use that as a cutoff time when filtering running jobs to cancel.
TODO:
Related to #3005.
RELEASE NOTES BEGIN
Packit now cancels running jobs even after manual retriggering.
RELEASE NOTES END