On Aug 3, 2009, at 9:07 AM, Nigel Kersten wrote: > > 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.
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. > >> >> 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 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
