Avoid checking a stale relationship #2949
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 introduces several valuable improvements to the database models. It correctly fixes a bug in CoprBuildTargetModel.get_all_by by changing the sorting from build_id (a string) to id (an integer), ensuring that the latest builds are retrieved correctly. This fix is well-supported by a new test case. Furthermore, the creation logic for TFTTestRunGroupModel is now more robust against race conditions. The changes, such as using session.flush() to get object IDs within a transaction and checking for existing relationships via foreign key IDs, are excellent practices for avoiding stale data. These modifications enhance the overall correctness and reliability of the database operations.
majamassarini
force-pushed
the
testing-farm-race-conditions-2
branch
from
8cfe963 to
1f95061
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 43s |
| new_run_model.copr_build_group = locked_run_model.copr_build_group | ||
| new_run_model.koji_build_group = locked_run_model.koji_build_group | ||
| new_run_model.test_run_group = test_run_group | ||
| new_run_model.test_run_group_id = test_run_group.id |
There was a problem hiding this comment.
Isn't this enough? Why keep the line above?
There was a problem hiding this comment.
I think the real problem is the if test. We end up on the wrong branch of the if. And the same PipelineModel line is overriden by the "second" task. But I may be wrong.
There was a problem hiding this comment.
Sure, I get that, but my comment is about recording the relationship (in either branch). Both lines do effectively the same thing, don't they?
There was a problem hiding this comment.
Oh ok, now I get your comment. Ok I think you are right, I think that removing the line above is safe.
majamassarini
force-pushed
the
testing-farm-race-conditions-2
branch
from
1f95061 to
2052492
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 45s |
|
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 1m 54s |
2 participants
No description provided.