Use pipeline datetime rather than job submission time to get newer job runs#3123
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
This pull request updates the has_newer_run logic across several models (CoprBuildTargetModel, KojiBuildTargetModel, and TFTTestRunTargetModel) to utilize the pipeline's datetime instead of the build's submission time. Corresponding updates were made to the test suite, including mocking the new logic and setting up environment variables. The review feedback correctly identifies that accessing runs[0] is non-deterministic in SQLAlchemy and recommends using the maximum datetime from all associated runs. Additionally, it suggests using datetime.utcnow() in tests to avoid issues with naive local datetimes.
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 50s |
…b runs Signed-off-by: Nikola Forró <nforro@redhat.com> Assisted-by: Claude Opus 4.6 via Claude Code
|
/gemini review |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 50s |
There was a problem hiding this comment.
Code Review
This pull request updates the has_newer_run logic across CoprBuildTargetModel, KojiBuildTargetModel, and TFTTestRunTargetModel to use a calculated pipeline_datetime instead of submitted_time. It also includes necessary test updates, such as adding datetime to the koji_build_pr_downstream mock and updating integration tests to mock has_newer_run. I have kept the feedback regarding the duplication of the pipeline_datetime calculation and the suggestion to use monkeypatch for modifying urls.DASHBOARD_URL in tests, as these provide actionable improvements for maintainability and test stability.
| runs = run.group_of_targets.runs if run.group_of_targets else [] | ||
| pipeline_datetime = max((r.datetime for r in runs if r.datetime), default=None) |
There was a problem hiding this comment.
This logic to calculate pipeline_datetime is duplicated in KojiBuildTargetModel.has_newer_run (line 3056) and TFTTestRunTargetModel.has_newer_run (line 3926). To improve maintainability and avoid future inconsistencies, consider extracting this logic into a common helper method in a follow-up change.
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 59s |
|
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 1m 52s |
github-project-automation
Bot
moved this from In review
to Done
in Packit pull requests
3 participants
No description provided.