Skip to content

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.

Related to #335814 (closed)

Edited by Dylan Griffith

Merge request reports

Loading