Ensure ci_pipelines.iid set before transaction in commit status API
What does this MR do and why?
Prior to this we were using create!
which triggered the
ensure_project_iid!
in a callback. This causes a
CrossDatabaseModificationAcrossUnsupportedTablesError
. To avoid this
we will create the iid
before saving. This is consistent with all
other places we create pipelines.
This change still doesn't allow us to remove
spec/requests/api/commit_statuses_spec.rb
from the allowlist as there
are other calls to project.ci_pipelines.create!
in this file.
We need a more holistic solution for that which is being discussed in !75316 (merged) .
It's important to note that this changes the semantics of what happens when saving the pipeline fails. Previously it would rollback the changes to the internal_ids
table but now that these have been moved out of the transaction this update to internal_ids
will not be rolled back. This isn't such a bad thing because it's just incrementing a counter and worse case scenario we have gaps in iid
. In some other parts of our application we do try to reset
this internal_id
but I find that doing this could be buggy and I don't see the benefit outweighing the cost.
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #335814 (closed)