Can you go into a bit more detail on the various repercussions of this?
I understand the idea, but we don't really do it anywhere else right
now, and I'm wondering if this is a theme we should start adopting, or
what.
If it is, then it should probably be in many places, not just this
one, right?
On Sep 3, 2009, at 10:57 AM, Steven Jenkins wrote:
>
> ---
> lib/puppet/indirector/facts/facter.rb | 8 +-------
> lib/puppet/node/facts.rb | 5 ++++-
> spec/unit/indirector/facts/facter.rb | 4 ++--
> spec/unit/node/facts.rb | 24 ++++++++++++++++--------
> 4 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/lib/puppet/indirector/facts/facter.rb b/lib/puppet/
> indirector/facts/facter.rb
> index 9df71fc..62ac3ed 100644
> --- a/lib/puppet/indirector/facts/facter.rb
> +++ b/lib/puppet/indirector/facts/facter.rb
> @@ -64,13 +64,7 @@ class Puppet::Node::Facts::Facter <
> Puppet::Indirector::Code
>
> # Look a host's facts up in Facter.
> def find(request)
> - result = Puppet::Node::Facts.new(request.key, Facter.to_hash)
> -
> - result.add_local_facts
> - result.stringify
> - result.downcase_if_necessary
> -
> - result
> + Puppet::Node::Facts.new(request.key,
> Facter.to_hash).add_local_facts.stringify.downcase_if_necessary
> end
>
> def save(facts)
> diff --git a/lib/puppet/node/facts.rb b/lib/puppet/node/facts.rb
> index dca435c..861214a 100755
> --- a/lib/puppet/node/facts.rb
> +++ b/lib/puppet/node/facts.rb
> @@ -24,6 +24,7 @@ class Puppet::Node::Facts
> def add_local_facts
> values["clientversion"] = Puppet.version.to_s
> values["environment"] ||= Puppet.settings[:environment]
> + self
> end
>
> def initialize(name, values = {})
> @@ -34,12 +35,13 @@ class Puppet::Node::Facts
> end
>
> def downcase_if_necessary
> - return unless Puppet.settings[:downcasefacts]
> + return self unless Puppet.settings[:downcasefacts]
>
> Puppet.warning "DEPRECATION NOTICE: Fact downcasing is
> deprecated; please disable (20080122)"
> values.each do |fact, value|
> values[fact] = value.downcase if value.is_a?(String)
> end
> + self
> end
>
> # Convert all fact values into strings.
> @@ -47,6 +49,7 @@ class Puppet::Node::Facts
> values.each do |fact, value|
> values[fact] = value.to_s
> end
> + self
> end
>
> private
> diff --git a/spec/unit/indirector/facts/facter.rb b/spec/unit/
> indirector/facts/facter.rb
> index ea68f63..9903912 100755
> --- a/spec/unit/indirector/facts/facter.rb
> +++ b/spec/unit/indirector/facts/facter.rb
> @@ -57,7 +57,7 @@ describe Puppet::Node::Facts::Facter do
> it "should add local facts" do
> facts = Puppet::Node::Facts.new("foo")
> Puppet::Node::Facts.expects(:new).returns facts
> - facts.expects(:add_local_facts)
> + facts.expects(:add_local_facts).returns facts
>
> @facter.find(@request)
> end
> @@ -65,7 +65,7 @@ describe Puppet::Node::Facts::Facter do
> it "should convert all facts into strings" do
> facts = Puppet::Node::Facts.new("foo")
> Puppet::Node::Facts.expects(:new).returns facts
> - facts.expects(:stringify)
> + facts.expects(:stringify).returns facts
>
> @facter.find(@request)
> end
> diff --git a/spec/unit/node/facts.rb b/spec/unit/node/facts.rb
> index a6e227a..44fb931 100755
> --- a/spec/unit/node/facts.rb
> +++ b/spec/unit/node/facts.rb
> @@ -9,26 +9,34 @@ describe Puppet::Node::Facts, "when indirecting" do
> @facts = Puppet::Node::Facts.new("me")
> end
>
> + it "add_local_facts should return an instance of
> Puppet::Node::Facts" do
> + @facts.add_local_facts.class.should == Puppet::Node::Facts
> + end
> +
> + it "stringify should return an instance of Puppet::Node::Facts"
> do
> + @facts.stringify.class.should == Puppet::Node::Facts
> + end
> +
> + it "downcase_if_necessary should return an instance of
> Puppet::Node::Facts" do
> + @facts.downcase_if_necessary.class.should ==
> Puppet::Node::Facts
> + end
> +
> it "should be able to convert all fact values to strings" do
> @facts.values["one"] = 1
> - @facts.stringify
> - @facts.values["one"].should == "1"
> + @facts.stringify.values["one"].should == "1"
> end
>
> it "should add the Puppet version as a 'clientversion' fact when
> adding local facts" do
> - @facts.add_local_facts
> - @facts.values["clientversion"].should == Puppet.version.to_s
> + @facts.add_local_facts.values["clientversion"].should ==
> Puppet.version.to_s
> end
>
> it "should add the current environment as a fact if one is not
> set when adding local facts" do
> - @facts.add_local_facts
> - @facts.values["environment"].should == Puppet[:environment]
> + @facts.add_local_facts.values["environment"].should ==
> Puppet[:environment]
> end
>
> it "should not replace any existing environment fact when adding
> local facts" do
> @facts.values["environment"] = "foo"
> - @facts.add_local_facts
> - @facts.values["environment"].should == "foo"
> + @facts.add_local_facts.values["environment"].should == "foo"
> end
>
> it "should be able to downcase fact values" do
> --
> 1.6.3.3
>
>
> >
--
I happen to feel that the degree of a person's intelligence is directly
reflected by the number of conflicting attitudes she can bring to bear
on the same topic. -- Lisa Alther
---------------------------------------------------------------------
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
-~----------~----~----~----~------~----~------~--~---
