WIP: Implement recovered pipeline emails
I don't understand why, but this is a new merge request from !8115 (merged)
I am thinking that perhaps we could take the advantage of detailed_status which we introduce Gitlab::Ci::Status::Pipeline::Recovered and we could put the implementation there and get the template back from each status. This would be awesome and we could also maybe do something with it in regular UI, if we want.
However, to detect if it's in recovered state, it would need to load the project and load the last pipeline from the same branch. I could probably reduce it to a single query, but still, every time we construct a detailed_status we'll need to walk through this. This doesn't sound good in terms of performance. I feel that implementing matches? should not involve any query at all, because this is a linear search and it could potentially slow many things down.
On the other hand, we could probably save this information in the database, so that we don't have to make a query to retrieve this information. However I am not sure if this is worth a column to store it.
Actually, SuccessWithWarnings already needs a query. It's probably ok to add another query for that? And we could optimize that later.
On the other hand, if we really have Recovered status, it could be weird that it's depending on the other pipeline, resulting the status changing due to the other pipeline changing.
@ayufan commented on Slack:
Lets try to avoid extra queries as you pointed out. We need to start storing more data in DB I believe. We tend to calculate to much in runtime which results in kind of terrible performance in any place that we present some CI data
🙂 By having a column:recovered, we could actually update that when we change a status of our pipeline tosuccess, it is easy and cheap
Should we really save recovered? I feel it's somehow redundant. status == 'success' contains part of the information already. Perhaps something like previous_pipeline_status would be more useful? But do we really need that? And we need a better name to avoid being confused about previous_status before retrying.
Now I started to remember why I gave up making this an extended status. We'll need to put email_template method into the status, otherwise it makes little point to have this recovered status. However core statuses were shared with builds, which doesn't need to have email_template, and I don't really want to add something like that to the core statuses.
Oops. I was hit into another issue. SuccessWithWarnings and Recovered could happen at the same time, of course.
Closes #24644 (closed)