Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Dynamic config #14

Closed
wants to merge 2 commits into from

5 participants

@lucaspinto

I don't want this to be merge but i'd like it be reviewed to know if the tests i wrote correspond to what we expect:
read the timeouts from the build payload if they are present.

@joshk
Owner

Just so I can make sure I understand, do we have hard defaults we make sure users can not exceed?

eg. a users says a script can have a timeout of 5 hours but we trim that to 1 hour, which is the max

@lucaspinto

This is not included yet. What should be the max values ? The default ones ?

@joshk
Owner

I think we need two ideas encapsulated in this PR.

The first is a 'default' set of timeouts eg. 5 mins for install, 20 mins for script

The second is a max custom timeout eg. if a project says 'please use my 5 hour timeout' the worker should say 'nice try but i am only going to allow for a 1 hour max'

The reason for the max is because if a project screws up and enters a way too big timeout but then hangs and also triggers lots of builds then the worker queue could be eaten up for ever requiring us to intervene.

Let me know what you think about these ideas.

J

@svenfuchs
Owner

i'd like to see a first "trusted" user to define a 5 hours timeout and actually use it. once this happens we can kill him from the whitelist, blame him on twitter and improve the feature.

@joshk
Owner
@rkh
Owner
rkh commented

group hug

@svenfuchs
Owner

Having looked over this with @lucaspinto yesterday we found this needs to be solved differently. Looking into it.

@kykim

what's the status on this?

@joshk
Owner

Closing as this PR is stale and so much has changed. Thanks for giving this a crack @lucaspinto, super high 5!

@joshk joshk closed this
@joshk joshk deleted the dynamic_config branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 22, 2012
  1. @svenfuchs
Commits on Mar 30, 2012
  1. @lucaspinto
This page is out of date. Refresh to see the latest.
View
2  features/support/step_definitions.rb
@@ -40,7 +40,7 @@ def decode(string)
end
When /^it starts a job$/ do
- $vm = Mocks::Vm.new
+ $vm = Mocks::Vm.new('vm-name', {})
$shell = Mocks::Shell.new
$observer = Mocks::Observer.new
$sequence = sequence('build')
View
8 lib/travis/build/factory.rb
@@ -29,7 +29,13 @@ def initialize(vm, shell, observers, payload, config)
end
def build
- build = configure? ? Build.new(events, job) : Build::Remote.new(vm, shell, events, job)
+ build = if configure?
+ Build.new(events, job)
+ else
+ # add dynamic configuration to the static vm config
+ vm.configure(payload.config)
+ Build::Remote.new(vm, shell, events, job)
+ end
observers.each { |observer| build.observers << observer }
build
end
View
12 spec/build/factory_spec.rb
@@ -7,9 +7,11 @@
# TODO check observers
describe Travis::Build::Factory do
- let(:vm) { stub('vm', :name => 'worker-1') }
+ let(:vm) { Mocks::Vm.new('vm-name', {}) }
let(:shell) { stub('shell') }
let(:observer) { stub('observer') }
+ let(:timeouts) { { :before_install => 42, :install => 42, :before_script => 42, :script => 42, :after_script => 42 } }
+ let(:payload) { { :config => { :timeouts => timeouts } } }
let(:config) { {} }
let(:build) { Travis::Build.create(vm, shell, [observer], payload, config) }
@@ -96,6 +98,12 @@
end
end
+ describe 'merges the payload config in the vm' do
+ it "has the given timeouts" do
+ build.vm.config.timeouts.should == timeouts
+ end
+ end
+
describe 'with "erlang" given as a language' do
before :each do
payload['config']['language'] = 'erlang'
@@ -154,7 +162,7 @@
end
it 'has the test config from the payload' do
- job.config.should == { :rvm => '1.9.2', :gemfile => 'Gemfile', :env => 'FOO=foo' }
+ job.config.should == { :rvm => '1.9.2', :gemfile => 'Gemfile', :env => 'FOO=foo', :timeouts => timeouts }
end
end
View
18 spec/spec_helper/mocks.rb
@@ -12,6 +12,20 @@ def notify(*args)
end
class Vm
+ attr_reader :name, :config, :shell
+
+ def initialize(name, config)
+ @name = name
+ @config = Hashr.new(config)
+ @shell = nil
+
+ if block_given?
+ connect
+ yield(self) if block_given?
+ close
+ end
+ end
+
def name
'vm-name'
end
@@ -23,6 +37,10 @@ def full_name
def sandboxed
yield
end
+
+ def configure(config)
+ self.config.merge!(config)
+ end
end
class Shell
View
4 spec/spec_helper/payloads.rb
@@ -8,12 +8,12 @@
'type' => 'test',
'repository' => { 'slug' => 'travis-ci/travis-ci', 'source_url' => 'git://github.com/travis-ci/travis-ci.git' },
'build' => { 'id' => 1, 'commit' => '313f61b' },
- 'config' => { 'rvm' => '1.9.2', 'env' => 'FOO=foo' }
+ 'config' => { 'rvm' => '1.9.2', 'env' => 'FOO=foo', 'timeouts' => { 'before_install' => 42, 'install' => 42, 'before_script' => 42, 'script' => 42, 'after_script' => 42 } }
},
:pull_request => {
'type' => 'test',
'repository' => { 'slug' => 'travis-ci/travis-ci', 'source_url' => 'git://github.com/travis-ci/travis-ci.git' },
'build' => { 'id' => 1, 'commit' => '313f61b', 'ref' => 'refs/pull/118/merge' },
- 'config' => { 'rvm' => '1.9.2', 'env' => 'FOO=foo' }
+ 'config' => { 'rvm' => '1.9.2', 'env' => 'FOO=foo', 'timeouts' => { 'before_install' => 42, 'install' => 42, 'before_script' => 42, 'script' => 42, 'after_script' => 42 } }
}
}
Something went wrong with that request. Please try again.