On Aug 3, 2009, at 9:54 AM, Nigel Kersten wrote: > > On Mon, Aug 3, 2009 at 9:28 AM, Luke Kanies<[email protected]> wrote: > >>>> I'm pretty sure this will cause you to always ignore report_server, >>>> depending on load order, because reportserver will always have a >>>> value, and this hook causes that value to get copied. >>> >>> So I've been checking the value inside the report indirector shows >>> it >>> using the value supplied in --report_server, and assumed that load >>> order would be in the order that the config file is laid out, but if >>> that isn't the case. >> >> It'd be the order in which they were set, which means it'd be the >> order in puppet.conf or on the command line. Not a good idea either >> way. >> >>>> You probably want to modify it so that call_on_define is off (the >>>> hook >>>> will thus get called only if a non-default value is provided). >>> >>> so *that* is what that does.... :) Definitely turning that off then. > > So whether I have call_on_define set or not, a simple integration > test like: > > Puppet.settings[:report_server] = "report_server" > Puppet.settings[:reportserver] = "reportserver" > Puppet.settings[:report_server].should == "report_server" > > will always fail if reportserver is set after report_server. It sounds > like this wasn't expected behavior?
Hmm, no, I think it is expected behaviour, now that I think about it. I think just with that call_on_define you might get some weird behaviour even if you didn't set 'reportserver'. >>>> It's also a good idea to have a couple of integration tests in >>>> spec/ >>>> integration/defaults.rb to verify the behaviour you want. >>> >>> so rather than doing the tests as below you'd rather they were in >>> that >>> test file? >> >> They're two kinds of tests - one is integration between the report >> terminii and the settings, the other is how the settings are used and >> propagated. >> >>>>> + :report_server => ["$server", >>>>> + "The server to which to send transacation reports." >>>>> + ], >>>>> + :report_port => ["$masterport", >>>>> + "The port to communicate with the report_server." >>>>> ], >>>>> :report => [false, >>>>> "Whether to send reports after every transaction." >>>>> diff --git a/lib/puppet/indirector/report/rest.rb b/lib/puppet/ >>>>> indirector/report/rest.rb >>>>> index 905b71a..f92d1ed 100644 >>>>> --- a/lib/puppet/indirector/report/rest.rb >>>>> +++ b/lib/puppet/indirector/report/rest.rb >>>>> @@ -2,4 +2,6 @@ require 'puppet/indirector/rest' >>>>> >>>>> class Puppet::Transaction::Report::Rest < Puppet::Indirector::REST >>>>> desc "Get server report over HTTP via REST." >>>>> + use_server_setting(:report_server) >>>>> + use_port_setting(:report_port) >>>>> end >>>>> diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb >>>>> index 6edc7f4..305bd2f 100644 >>>>> --- a/lib/puppet/util/log.rb >>>>> +++ b/lib/puppet/util/log.rb >>>>> @@ -514,7 +514,7 @@ class Puppet::Util::Log >>>>> # We can't store the actual source, we just store the >>>>> path. >>>>> # We can't just check for whether it responds to :path, >>>>> because >>>>> # plenty of providers respond to that in their normal >>>>> function. >>>>> - if (source.is_a?(Puppet::Type) or source.is_a? >>>>> (Puppet::Parameter)) and source.respond_to?(:path) >>>>> + if defined?(Puppet::Type) and (source.is_a?(Puppet::Type) >>>>> or source.is_a?(Puppet::Parameter)) and source.respond_to?(:path) >>>>> @source = source.path >>>>> else >>>>> @source = source.to_s >>>>> diff --git a/spec/unit/indirector/report/rest.rb b/spec/unit/ >>>>> indirector/report/rest.rb >>>>> old mode 100644 >>>>> new mode 100755 >>>>> index a51ebca..1f71eb3 >>>>> --- a/spec/unit/indirector/report/rest.rb >>>>> +++ b/spec/unit/indirector/report/rest.rb >>>>> @@ -5,7 +5,24 @@ require File.dirname(__FILE__) + '/../../../ >>>>> spec_helper' >>>>> require 'puppet/indirector/report/rest' >>>>> >>>>> describe Puppet::Transaction::Report::Rest do >>>>> - it "should be a sublcass of Puppet::Indirector::REST" do >>>>> + it "should be a subclass of Puppet::Indirector::REST" do >>>>> Puppet::Transaction::Report::Rest.superclass.should >>>>> equal(Puppet::Indirector::REST) >>>>> end >>>>> + >>>>> + it "should use the :report_server setting in preference >>>>> to :reportserver" do >>>>> + Puppet.settings[:reportserver] = "reportserver" >>>>> + Puppet.settings[:report_server] = "report_server" >>>>> + Puppet::Transaction::Report::Rest.server.should == >>>>> "report_server" >>>>> + end >>>>> + >>>>> + it "should use the :report_server setting in preference >>>>> to :server" do >>>>> + Puppet.settings[:server] = "server" >>>>> + Puppet.settings[:report_server] = "report_server" >>>>> + Puppet::Transaction::Report::Rest.server.should == >>>>> "report_server" >>>>> + end >>>>> + >>>>> + it "should have a value for report_server and report_port" do >>>>> + Puppet::Transaction::Report::Rest.server.should_not >>>>> be_nil >>>>> + Puppet::Transaction::Report::Rest.port.should_not be_nil >>>>> + end >>>>> end >>>>> -- >>>>> 1.6.3.3 >>>>> >>>>> >>>>>> >>>> >>>> >>>> -- >>>> Everything that is really great and inspiring is created by the >>>> individual who can labor in freedom. -- Albert Einstein >>>> --------------------------------------------------------------------- >>>> Luke Kanies | http://reductivelabs.com | http://madstop.com >>>> >>>> >>>>> >>>> >>> >>> >>> >>> -- >>> Nigel Kersten >>> [email protected] >>> System Administrator >>> Google, Inc. >>> >>>> >> >> >> -- >> It's not to control, but to protect the citizens of Singapore. In our >> society, you can state your views, but they have to be correct. >> -- Ernie Hai, co-ordinator of the Singapore Government >> Internet Project >> --------------------------------------------------------------------- >> Luke Kanies | http://reductivelabs.com | http://madstop.com >> >> >>> >> > > > > -- > Nigel Kersten > [email protected] > System Administrator > Google, Inc. > > > -- Patriotism is often an arbitrary veneration of real estate above principles. -- George Jean Nathan --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
