Remove PagesService and instead make it explicit that we call PagesWorker
This commit is contained in:
parent
f12ee2a2f4
commit
b009a0084c
|
@ -144,6 +144,7 @@ module Ci
|
|||
after_transition any => [:success] do |build|
|
||||
build.run_after_commit do
|
||||
BuildSuccessWorker.perform_async(id)
|
||||
PagesWorker.perform_async(:deploy, id) if build.pages_generator?
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -183,6 +184,11 @@ module Ci
|
|||
pipeline.manual_actions.where.not(name: name)
|
||||
end
|
||||
|
||||
def pages_generator?
|
||||
Gitlab.config.pages.enabled &&
|
||||
self.name == 'pages'
|
||||
end
|
||||
|
||||
def playable?
|
||||
action? && (manual? || retryable?)
|
||||
end
|
||||
|
@ -402,7 +408,6 @@ module Ci
|
|||
build_data = Gitlab::DataBuilder::Build.build(self)
|
||||
project.execute_hooks(build_data.dup, :job_hooks)
|
||||
project.execute_services(build_data.dup, :job_hooks)
|
||||
PagesService.new(build_data).execute
|
||||
end
|
||||
|
||||
def browsable_artifacts?
|
||||
|
|
|
@ -1,15 +0,0 @@
|
|||
class PagesService
|
||||
attr_reader :data
|
||||
|
||||
def initialize(data)
|
||||
@data = data
|
||||
end
|
||||
|
||||
def execute
|
||||
return unless Settings.pages.enabled
|
||||
return unless data[:build_name] == 'pages'
|
||||
return unless data[:build_status] == 'success'
|
||||
|
||||
PagesWorker.perform_async(:deploy, data[:build_id])
|
||||
end
|
||||
end
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Optimise PagesWorker usage
|
||||
merge_request:
|
||||
author:
|
||||
type: performance
|
|
@ -2506,4 +2506,76 @@ describe Ci::Build do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'pages deployments' do
|
||||
set(:build) { create(:ci_build, project: project, user: user) }
|
||||
|
||||
context 'when job is "pages"' do
|
||||
before do
|
||||
build.name = 'pages'
|
||||
end
|
||||
|
||||
context 'when pages are enabled' do
|
||||
before do
|
||||
allow(Gitlab.config.pages).to receive_messages(enabled: true)
|
||||
end
|
||||
|
||||
it 'is marked as pages generator' do
|
||||
expect(build).to be_pages_generator
|
||||
end
|
||||
|
||||
context 'job succeeds' do
|
||||
it "calls pages worker" do
|
||||
expect(PagesWorker).to receive(:perform_async).with(:deploy, build.id)
|
||||
|
||||
build.success!
|
||||
end
|
||||
end
|
||||
|
||||
context 'job fails' do
|
||||
it "does not call pages worker" do
|
||||
expect(PagesWorker).not_to receive(:perform_async)
|
||||
|
||||
build.drop!
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when pages are disabled' do
|
||||
before do
|
||||
allow(Gitlab.config.pages).to receive_messages(enabled: false)
|
||||
end
|
||||
|
||||
it 'is not marked as pages generator' do
|
||||
expect(build).not_to be_pages_generator
|
||||
end
|
||||
|
||||
context 'job succeeds' do
|
||||
it "does not call pages worker" do
|
||||
expect(PagesWorker).not_to receive(:perform_async)
|
||||
|
||||
build.success!
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when job is not "pages"' do
|
||||
before do
|
||||
build.name = 'other-job'
|
||||
end
|
||||
|
||||
it 'is not marked as pages generator' do
|
||||
expect(build).not_to be_pages_generator
|
||||
end
|
||||
|
||||
context 'job succeeds' do
|
||||
it "does not call pages worker" do
|
||||
expect(PagesWorker).not_to receive(:perform_async)
|
||||
|
||||
build.success
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,53 +0,0 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe PagesService do
|
||||
let(:build) { create(:ci_build) }
|
||||
let(:data) { Gitlab::DataBuilder::Build.build(build) }
|
||||
let(:service) { described_class.new(data) }
|
||||
|
||||
before do
|
||||
allow(Gitlab.config.pages).to receive(:enabled).and_return(true)
|
||||
end
|
||||
|
||||
context 'execute asynchronously for pages job' do
|
||||
before do
|
||||
build.name = 'pages'
|
||||
end
|
||||
|
||||
context 'on success' do
|
||||
before do
|
||||
build.success
|
||||
end
|
||||
|
||||
it 'executes worker' do
|
||||
expect(PagesWorker).to receive(:perform_async)
|
||||
service.execute
|
||||
end
|
||||
end
|
||||
|
||||
%w(pending running failed canceled).each do |status|
|
||||
context "on #{status}" do
|
||||
before do
|
||||
build.status = status
|
||||
end
|
||||
|
||||
it 'does not execute worker' do
|
||||
expect(PagesWorker).not_to receive(:perform_async)
|
||||
service.execute
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'for other jobs' do
|
||||
before do
|
||||
build.name = 'other job'
|
||||
build.success
|
||||
end
|
||||
|
||||
it 'does not execute worker' do
|
||||
expect(PagesWorker).not_to receive(:perform_async)
|
||||
service.execute
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue