Pipelines page shows invalid YAML errors in front of later pipelines
In gitlab-org/ruby/gems/grpc!1 (merged), I've noticed that pipelines with invalid YAML remain at the top, while later pipelines are under them:
I think this has to with the sorting in https://gitlab.com/gitlab-org/gitlab/blob/4ced63e8cc36ab0a5c4b5f06a2b947f84fa289d4/app/finders/ci/pipelines_for_merge_request_finder.rb#L108-109:
def sort(pipelines)
sql = 'CASE ci_pipelines.source WHEN (?) THEN 0 ELSE 1 END, ci_pipelines.id DESC'
query = ApplicationRecord.send(:sanitize_sql_array, [sql, Ci::Pipeline.sources[:merge_request_event]]) # rubocop:disable GitlabSecurity/PublicSend
pipelines.order(Arel.sql(query)) # rubocop: disable CodeReuse/ActiveRecord
end
If we look at this:
pipeline = Ci::Pipeline.find(401388790)
mr = pipeline.merge_request
finder = Ci::PipelinesForMergeRequestFinder.new(mr, User.find_by(username: 'stanhu'))
pipes = finder.execute
We see this SQL query:
[ gprd ] production> pipes = finder.execute
D, [2021-11-03T18:41:51.092400 #7458] DEBUG -- : Ci::Pipeline Load (7.7ms) /*application:console,db_config_name:main_replica*/ WITH "shas" AS MATERIALIZED (SELECT DISTINCT "merge_request_diff_commits"."sha" FROM "merge_request_diff_commits" WHERE "merge_request_diff_commits"."merge_request_diff_id" IN (SELECT "merge_request_diffs"."id" FROM "merge_request_diffs" WHERE "merge_request_diffs"."merge_request_id" = 124358291 AND "merge_request_diffs"."diff_type" = 1 ORDER BY "merge_request_diffs"."id" DESC LIMIT 100) LIMIT 10000) SELECT "ci_pipelines".* FROM ((SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."source" = 10 AND "ci_pipelines"."merge_request_id" = 124358291 AND "ci_pipelines"."project_id" IN (31017252, 31017252))
UNION
(SELECT "ci_pipelines".* FROM "ci_pipelines" INNER JOIN "shas" ON encode("shas"."sha", 'hex') = "ci_pipelines"."sha" WHERE "ci_pipelines"."project_id" = 31017252 AND ("ci_pipelines"."source" IN (1, 2, 3, 4, 5, 6, 7, 8, 11) OR "ci_pipelines"."source" IS NULL) AND "ci_pipelines"."ref" = 'sh-gitlab-v1.30.x' AND "ci_pipelines"."tag" = FALSE)) ci_pipelines /* loading for inspect */ ORDER BY CASE ci_pipelines.source WHEN (10) THEN 0 ELSE 1 END, ci_pipelines.id DESC LIMIT 11
Notice that the ci_pipelines.source
takes precedence. In the failed pipeline case, we have a merge_request_event
, but the other pipelines underneath are related to push
:
[ gprd ] production> pipes.map(&:source)
=> ["merge_request_event", "push", "push", "push", "push", "push"]
@fabiopitino @shinya.maeda Should we swap the ordering of the ci_pipelines.source
and ci_pipelines.id
here?
Edited by Stan Hu