Use persisted stages to load pipelines index table
This commit is contained in:
parent
b113330fab
commit
631bd9bf08
9 changed files with 72 additions and 21 deletions
|
@ -1,5 +1,5 @@
|
|||
class Projects::PipelinesController < Projects::ApplicationController
|
||||
before_action :whitelist_query_limiting, only: [:create, :retry]
|
||||
# before_action :whitelist_query_limiting, only: [:create, :retry]
|
||||
before_action :pipeline, except: [:index, :new, :create, :charts]
|
||||
before_action :commit, only: [:show, :builds, :failures]
|
||||
before_action :authorize_read_pipeline!
|
||||
|
@ -15,6 +15,7 @@ class Projects::PipelinesController < Projects::ApplicationController
|
|||
@pipelines = PipelinesFinder
|
||||
.new(project, scope: @scope)
|
||||
.execute
|
||||
.preload(:stages)
|
||||
.page(params[:page])
|
||||
.per(30)
|
||||
|
||||
|
@ -23,7 +24,7 @@ class Projects::PipelinesController < Projects::ApplicationController
|
|||
@finished_count = limited_pipelines_count(project, 'finished')
|
||||
@pipelines_count = limited_pipelines_count(project)
|
||||
|
||||
Gitlab::Ci::Pipeline::Preloader.preload(@pipelines)
|
||||
Gitlab::Ci::Pipeline::Preloader.preload(@project, @pipelines)
|
||||
|
||||
respond_to do |format|
|
||||
format.html
|
||||
|
|
|
@ -13,7 +13,7 @@ module Ci
|
|||
belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline'
|
||||
belongs_to :pipeline_schedule, class_name: 'Ci::PipelineSchedule'
|
||||
|
||||
has_many :stages
|
||||
has_many :stages, inverse_of: :pipeline # -> { order(position: :asc) }, inverse_of: :pipeline
|
||||
has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline
|
||||
has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline
|
||||
has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent
|
||||
|
|
|
@ -79,5 +79,23 @@ module Ci
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
def groups
|
||||
@groups ||= statuses.ordered.latest
|
||||
.sort_by(&:sortable_name).group_by(&:group_name)
|
||||
.map do |group_name, grouped_statuses|
|
||||
Ci::Group.new(self, name: group_name, jobs: grouped_statuses)
|
||||
end
|
||||
end
|
||||
|
||||
def has_warnings?
|
||||
statuses.latest.failed_but_allowed.any?
|
||||
end
|
||||
|
||||
def detailed_status(current_user)
|
||||
Gitlab::Ci::Status::Stage::Factory
|
||||
.new(self, current_user)
|
||||
.fabricate!
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -227,6 +227,7 @@ class Project < ActiveRecord::Base
|
|||
|
||||
has_many :commit_statuses
|
||||
has_many :pipelines, class_name: 'Ci::Pipeline', inverse_of: :project
|
||||
has_many :stages, class_name: 'Ci::Stage', inverse_of: :project
|
||||
|
||||
# Ci::Build objects store data on the file system such as artifact files and
|
||||
# build traces. Currently there's no efficient way of removing this data in
|
||||
|
|
|
@ -1,6 +1,11 @@
|
|||
class PipelineDetailsEntity < PipelineEntity
|
||||
expose :details do
|
||||
expose :legacy_stages, as: :stages, using: StageEntity
|
||||
##
|
||||
# TODO consider switching to persisted stages only in pipelines table
|
||||
# (not necessairly in the show pipeline page because of #23257.
|
||||
# Hide this behind two feature flags - enabled / disabled and only
|
||||
# gitlab-ce / everywhere.
|
||||
expose :stages, as: :stages, using: StageEntity
|
||||
expose :artifacts, using: BuildArtifactEntity
|
||||
expose :manual_actions, using: BuildActionEntity
|
||||
end
|
||||
|
|
|
@ -6,7 +6,7 @@ module Gitlab
|
|||
# Class for preloading data associated with pipelines such as commit
|
||||
# authors.
|
||||
module Preloader
|
||||
def self.preload(pipelines)
|
||||
def self.preload(project, pipelines)
|
||||
# This ensures that all the pipeline commits are eager loaded before we
|
||||
# start using them.
|
||||
pipelines.each(&:commit)
|
||||
|
|
|
@ -8,7 +8,9 @@ module Gitlab
|
|||
end
|
||||
|
||||
def details_path
|
||||
project_pipeline_path(subject.project, subject.pipeline, anchor: subject.name)
|
||||
project_pipeline_path(subject.pipeline.project,
|
||||
subject.pipeline,
|
||||
anchor: subject.name)
|
||||
end
|
||||
|
||||
def has_action?
|
||||
|
|
|
@ -47,6 +47,8 @@ module Gitlab
|
|||
|
||||
# Sends a notification based on the number of executed SQL queries.
|
||||
def act_upon_results
|
||||
puts "XXXX\n\n\n\n\n #{count} \n\n\nXXXX"
|
||||
|
||||
return unless threshold_exceeded?
|
||||
|
||||
error = ThresholdExceededError.new(error_message)
|
||||
|
|
|
@ -19,16 +19,21 @@ describe Projects::PipelinesController do
|
|||
before do
|
||||
%w(pending running created success).each_with_index do |status, index|
|
||||
sha = project.commit("HEAD~#{index}")
|
||||
create(:ci_empty_pipeline, status: status, project: project, sha: sha)
|
||||
|
||||
pipeline = create(:ci_empty_pipeline, status: status,
|
||||
project: project,
|
||||
sha: sha)
|
||||
|
||||
|
||||
create_build(pipeline, 'test', 1, 'unit')
|
||||
create_build(pipeline, 'test', 1, 'feature')
|
||||
create_build(pipeline, 'review', 2, 'staging')
|
||||
create_build(pipeline, 'deploy', 3, 'production')
|
||||
end
|
||||
end
|
||||
|
||||
subject do
|
||||
get :index, namespace_id: project.namespace, project_id: project, format: :json
|
||||
end
|
||||
|
||||
it 'returns JSON with serialized pipelines' do
|
||||
subject
|
||||
it 'returns JSON with serialized pipelines', :request_store do
|
||||
queries = ActiveRecord::QueryRecorder.new { get_pipelines_index_json }
|
||||
|
||||
expect(response).to have_gitlab_http_status(:ok)
|
||||
expect(response).to match_response_schema('pipeline')
|
||||
|
@ -39,22 +44,35 @@ describe Projects::PipelinesController do
|
|||
expect(json_response['count']['running']).to eq '1'
|
||||
expect(json_response['count']['pending']).to eq '1'
|
||||
expect(json_response['count']['finished']).to eq '1'
|
||||
puts queries.log
|
||||
expect(queries.count).to be < 25
|
||||
end
|
||||
|
||||
it 'does not include coverage data for the pipelines' do
|
||||
subject
|
||||
get_pipelines_index_json
|
||||
|
||||
expect(json_response['pipelines'][0]).not_to include('coverage')
|
||||
end
|
||||
|
||||
context 'when performing gitaly calls', :request_store do
|
||||
it 'limits the Gitaly requests' do
|
||||
expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3)
|
||||
expect { get_pipelines_index_json }
|
||||
.to change { Gitlab::GitalyClient.get_request_count }.by(2)
|
||||
end
|
||||
end
|
||||
|
||||
def get_pipelines_index_json
|
||||
get :index, namespace_id: project.namespace,
|
||||
project_id: project,
|
||||
format: :json
|
||||
end
|
||||
|
||||
def create_build(pipeline, stage, stage_idx, name)
|
||||
create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'GET show JSON' do
|
||||
describe 'GET show.json' do
|
||||
let(:pipeline) { create(:ci_pipeline_with_one_job, project: project) }
|
||||
|
||||
it 'returns the pipeline' do
|
||||
|
@ -67,6 +85,14 @@ describe Projects::PipelinesController do
|
|||
end
|
||||
|
||||
context 'when the pipeline has multiple stages and groups', :request_store do
|
||||
let(:project) { create(:project, :repository) }
|
||||
|
||||
let(:pipeline) do
|
||||
create(:ci_empty_pipeline, project: project,
|
||||
user: user,
|
||||
sha: project.commit.id)
|
||||
end
|
||||
|
||||
before do
|
||||
create_build('build', 0, 'build')
|
||||
create_build('test', 1, 'rspec 0')
|
||||
|
@ -74,11 +100,6 @@ describe Projects::PipelinesController do
|
|||
create_build('post deploy', 3, 'pages 0')
|
||||
end
|
||||
|
||||
let(:project) { create(:project, :repository) }
|
||||
let(:pipeline) do
|
||||
create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id)
|
||||
end
|
||||
|
||||
it 'does not perform N + 1 queries' do
|
||||
control_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count
|
||||
|
||||
|
@ -90,6 +111,7 @@ describe Projects::PipelinesController do
|
|||
create_build('post deploy', 3, 'pages 2')
|
||||
|
||||
new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count
|
||||
|
||||
expect(new_count).to be_within(12).of(control_count)
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue