Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configuring a per-repo concurrency limit via config #75

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/travis/scheduler/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Config < Travis::Config
enterprise: false,
github: { api_url: 'https://api.github.com', source_host: 'github.com' },
interval: 2,
limit: { default: 5, by_owner: {}, delegate: {} },
limit: { default: 5, by_owner: {}, by_repo: {}, delegate: {} },
lock: { strategy: :redis, ttl: 150 },
logger: { time_format: false, process_id: false, thread_id: false },
log_level: :info,
Expand Down
26 changes: 19 additions & 7 deletions lib/travis/scheduler/limit/by_repo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ class ByRepo < Struct.new(:context, :owners, :job, :selected, :state, :config)
include Helper::Context

def enqueue?
unlimited? || by_settings
# return false unless by_config
# return false if max_by_setting && !by_setting
# p [max_by_setting, by_setting]
# unlimited? || by_setting
return false if limited_by_config?
return false if limited_by_setting?
true
end

def reports
Expand All @@ -17,24 +23,30 @@ def reports

private

def unlimited?
max == 0
def limited_by_config?
result = max_by_config > 0 && current >= max_by_config
report :repo_config, max_by_config if result
result
end

def by_settings
result = current < max
report :repo_settings, max unless result
def limited_by_setting?
result = max_by_setting > 0 && current >= max_by_setting
report :repo_settings, max_by_setting if result
result
end

def current
state.running_by_repo(repo.id) + selected.select { |j| j.repository_id == repo.id }.size
end

def max
def max_by_setting
repo.settings.maximum_number_of_builds.to_i
end

def max_by_config
config[:limit][:by_repo][repo.slug].to_i
end

def repo
job.repository
end
Expand Down
4 changes: 4 additions & 0 deletions lib/travis/scheduler/limit/by_stage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ class ByStage < Struct.new(:context, :owners, :job, :queued, :state, :config)
def enqueue?
return true unless job.stage_number
!!report if queueable?
# return true unless job.stage_number
# throw :result, :limited unless queueable?
# report
# true
end

def reports
Expand Down
9 changes: 6 additions & 3 deletions lib/travis/scheduler/limit/jobs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,14 @@ def check(job)
end

def enqueue?(job)
limits_for(job).map do |limit|
result = limit.enqueue?
a = limits_for(job).map do |limit|
result = catch(:result) { limit.enqueue? }
report *limit.reports
throw :result, result if result == :limited
result
end.inject(&:&)
end
# p a
a.inject(&:&)
end

def limits_for(job)
Expand Down
44 changes: 44 additions & 0 deletions spec/travis/scheduler/limit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ def create_jobs(count, attrs = {})
it { expect(report).to include('user svenfuchs: total: 3, running: 0, queueable: 1') }
end

describe 'with a default limit 5 and a repo keychain/config limit 2' do
before { config.limit.default = 5 }
before { config.limit.by_repo = { repo.slug => 2 } }
before { create_jobs(3) }
before { repo.settings.update_attributes!(maximum_number_of_builds: 2) }

This comment was marked as spam.

before { subject }

it { expect(subject.size).to eq 2 }
it { expect(report).to include('max jobs for repo svenfuchs/gem-release by repo_config: 2') }
it { expect(report).to include('user svenfuchs: total: 3, running: 0, queueable: 2') }
end

describe 'with a default limit 5 and a repo settings limit 2' do
before { config.limit.default = 5 }
before { create_jobs(3) }
Expand Down Expand Up @@ -193,4 +205,36 @@ def create_jobs(count, attrs = {})
it { expect(subject.size).to eq 2 }
it { expect(report).to include("jobs for build #{build.id} limited at stage: 1 (queueable: 2)") }
end

describe 'stages with a a repo config limit 1' do
let(:one) { FactoryGirl.create(:stage, number: 1) }
let(:two) { FactoryGirl.create(:stage, number: 2) }

before { create_jobs(1, owner: owner, state: :created, stage: one, stage_number: '1.1') }
before { create_jobs(1, owner: owner, state: :created, stage: one, stage_number: '1.2') }
before { create_jobs(1, owner: owner, state: :created, stage: two, stage_number: '2.1') }
before { config.limit.default = 5 }
before { config.limit.by_repo = { repo.slug => 1 } }
before { subject }

it { expect(subject.size).to eq 1 }
it { expect(report).to include("jobs for build #{build.id} limited at stage: 1 (queueable: 2)") }
it { expect(report).to include('max jobs for repo svenfuchs/gem-release by repo_config: 1') }
end

describe 'stages with a a repo settings limit 5' do
let(:one) { FactoryGirl.create(:stage, number: 1) }
let(:two) { FactoryGirl.create(:stage, number: 2) }

before { create_jobs(1, owner: owner, state: :created, stage: one, stage_number: '1.1') }
before { create_jobs(1, owner: owner, state: :created, stage: one, stage_number: '1.2') }
before { create_jobs(1, owner: owner, state: :created, stage: two, stage_number: '2.1') }
before { config.limit.default = 5 }
before { repo.settings.update_attributes!(maximum_number_of_builds: 1) }
before { subject }

it { expect(subject.size).to eq 1 }
it { expect(report).to include("jobs for build #{build.id} limited at stage: 1 (queueable: 2)") }
it { expect(report).to include('max jobs for repo svenfuchs/gem-release by repo_settings: 1') }
end
end