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?
>>
>>>
>>> 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.
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
