Skip to content

Fix setting cutoff time for TF tasks triggered by finished Copr build#3096

Merged
centosinfra-prod-github-app[bot] merged 1 commit intopackit:mainfrom
nforro:canceling
Apr 8, 2026
Merged

Fix setting cutoff time for TF tasks triggered by finished Copr build#3096
centosinfra-prod-github-app[bot] merged 1 commit intopackit:mainfrom
nforro:canceling

Conversation

@nforro
Copy link
Copy Markdown
Member

@nforro nforro commented Apr 8, 2026

Related to #3076.

Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code
@nforro nforro requested a review from a team as a code owner April 8, 2026 10:15
@nforro nforro requested review from betulependule and removed request for a team April 8, 2026 10:15
@nforro nforro moved this from New to In review in Packit pull requests Apr 8, 2026
Copy link
Copy Markdown
Member

@majamassarini majamassarini left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the handle_testing_farm method in packit_service/worker/handlers/copr.py to calculate the cancel_cutoff_time based on the earliest pipeline datetime in the current build batch, rather than just the first run. It also introduces a one-second buffer to ensure sibling tests from the same batch are not canceled. A review comment pointed out a potential issue where calling .timestamp() on naive datetime objects might use the local system timezone instead of UTC, suggesting an explicit conversion to UTC to ensure consistency across different server environments.

default=None,
)
if earliest:
event_dict["cancel_cutoff_time"] = (earliest - timedelta(seconds=1)).timestamp()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The PipelineModel.datetime column stores naive datetime objects representing UTC. When calling .timestamp() on a naive datetime object, Python assumes it is in the local system timezone. If the server environment is not configured to UTC, this will result in an incorrect timestamp offset. It is safer to explicitly set the timezone to UTC before calculating the timestamp.

Suggested change
event_dict["cancel_cutoff_time"] = (earliest - timedelta(seconds=1)).timestamp()
event_dict["cancel_cutoff_time"] = (
earliest.replace(tzinfo=timezone.utc) - timedelta(seconds=1)
).timestamp()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think this matters here.

@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

@nforro nforro added the mergeit Merge via Zuul label Apr 8, 2026
@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

@centosinfra-prod-github-app centosinfra-prod-github-app bot merged commit ec8bde2 into packit:main Apr 8, 2026
6 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Packit pull requests Apr 8, 2026
@nforro nforro deleted the canceling branch April 8, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mergeit Merge via Zuul

Projects

Development

Successfully merging this pull request may close these issues.

3 participants