On Mon, Aug 3, 2009 at 8:58 AM, Luke Kanies<[email protected]> wrote: > > On Aug 3, 2009, at 8:21 AM, Nigel Kersten wrote: > >> >> >> Signed-off-by: Nigel Kersten <[email protected]> >> --- >> lib/puppet/defaults.rb | 17 +++++++++++++++-- >> lib/puppet/indirector/report/rest.rb | 2 ++ >> lib/puppet/util/log.rb | 2 +- >> spec/unit/indirector/report/rest.rb | 19 ++++++++++++++++++- >> 4 files changed, 36 insertions(+), 4 deletions(-) >> mode change 100644 => 100755 spec/unit/indirector/report/rest.rb >> >> diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb >> index b1fddc3..5383224 100644 >> --- a/lib/puppet/defaults.rb >> +++ b/lib/puppet/defaults.rb >> @@ -542,8 +542,21 @@ module Puppet >> before considering it a failure. This can help reduce >> flapping if too >> many clients contact the server at one time." >> ], >> - :reportserver => ["$server", >> - "The server to which to send transaction reports." >> + :reportserver => { >> + :default => "$server", >> + :call_on_define => true, >> + :desc => "(Deprecated for 'report_server') The server >> to which to send transaction reports.", >> + :hook => proc do |value| >> + if value >> + Puppet.settings[:report_server] = value >> + end >> + end >> + }, > > 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. > > 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. > > 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? > >> >> + :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. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
