+2!

On Mon, Jul 19, 2010 at 6:03 PM, Luke Kanies <[email protected]> wrote:
> The problem is that the environment list gets cleared
> when Settings#set_value is called, and it was being
> called every time Settings#use was called, which is
> more often than obvious but especially if reporting
> is enabled.
>
> Previously we ignored noop when running inside of Settings,
> and this essentially adds that back in, so we can remove
> the special noop behaviour in Settings itself.
>
> Signed-off-by: Luke Kanies <[email protected]>
> ---
>  lib/puppet/type.rb              |    4 ++++
>  lib/puppet/util/settings.rb     |   21 ++++++---------------
>  spec/unit/type_spec.rb          |   16 ++++++++++++++++
>  spec/unit/util/settings_spec.rb |   23 -----------------------
>  4 files changed, 26 insertions(+), 38 deletions(-)
>
> diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
> index 8807110..c3855a4 100644
> --- a/lib/puppet/type.rb
> +++ b/lib/puppet/type.rb
> @@ -723,6 +723,10 @@ class Type
>
>   # Are we running in noop mode?
>   def noop?
> +    # If we're not a host_config, we're almost certainly part of
> +    # Settings, and we want to ignore 'noop'
> +    return false if catalog and ! catalog.host_config?
> +
>     if defined?(@noop)
>       @noop
>     else
> diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
> index cbb12a8..ca4ecda 100644
> --- a/lib/puppet/util/settings.rb
> +++ b/lib/puppet/util/settings.rb
> @@ -157,13 +157,6 @@ class Puppet::Util::Settings
>     set_value(str, value, :cli)
>   end
>
> -  def without_noop
> -    old_noop = value(:noop,:cli) and set_value(:noop, false, :cli) if 
> valid?(:noop)
> -    yield
> -  ensure
> -    set_value(:noop, old_noop, :cli) if valid?(:noop)
> -  end
> -
>   def include?(name)
>     name = name.intern if name.is_a? String
>     @config.include?(name)
> @@ -635,14 +628,12 @@ if @config.include?(:run_mode)
>         return
>       end
>
> -      without_noop do
> -        catalog.host_config = false
> -        catalog.apply do |transaction|
> -          if transaction.any_failed?
> -            report = transaction.report
> -            failures = report.logs.find_all { |log| log.level == :err }
> -            raise "Got #{failures.length} failure(s) while initializing: 
> #{failures.collect { |l| l.to_s }.join("; ")}"
> -          end
> +      catalog.host_config = false
> +      catalog.apply do |transaction|
> +        if transaction.any_failed?
> +          report = transaction.report
> +          failures = report.logs.find_all { |log| log.level == :err }
> +          raise "Got #{failures.length} failure(s) while initializing: 
> #{failures.collect { |l| l.to_s }.join("; ")}"
>         end
>       end
>
> diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb
> index 71d415d..487750e 100755
> --- a/spec/unit/type_spec.rb
> +++ b/spec/unit/type_spec.rb
> @@ -123,6 +123,22 @@ describe Puppet::Type do
>     Puppet::Type.type(:mount).new(:name => "foo").type.should == :mount
>   end
>
> +  it "should use any provided noop value" do
> +    Puppet::Type.type(:mount).new(:name => "foo", :noop => true).must be_noop
> +  end
> +
> +  it "should use the global noop value if none is provided" do
> +    Puppet[:noop] = true
> +    Puppet::Type.type(:mount).new(:name => "foo").must be_noop
> +  end
> +
> +  it "should not be noop if in a non-host_config catalog" do
> +    resource = Puppet::Type.type(:mount).new(:name => "foo")
> +    catalog = Puppet::Resource::Catalog.new
> +    catalog.add_resource resource
> +    resource.should_not be_noop
> +  end
> +
>   describe "when creating an event" do
>     before do
>       @resource = Puppet::Type.type(:mount).new :name => "foo"
> diff --git a/spec/unit/util/settings_spec.rb b/spec/unit/util/settings_spec.rb
> index 81cab79..7bca44b 100755
> --- a/spec/unit/util/settings_spec.rb
> +++ b/spec/unit/util/settings_spec.rb
> @@ -1096,27 +1096,4 @@ describe Puppet::Util::Settings do
>
>     it "should cache the result"
>   end
> -
> -  describe "#without_noop" do
> -    before do
> -     �...@settings = Puppet::Util::Settings.new
> -     �[email protected] :main, :noop => [true, ""]
> -    end
> -
> -    it "should set noop to false for the duration of the block" do
> -     �[email protected]_noop { @settings.value(:noop, :cli).should 
> be_false }
> -    end
> -
> -    it "should ensure that noop is returned to its previous value" do
> -     �[email protected]_noop { raise } rescue nil
> -     �[email protected](:noop, :cli).should be_true
> -    end
> -
> -    it "should work even if no 'noop' setting is available" do
> -      settings = Puppet::Util::Settings.new
> -      stuff = nil
> -      settings.without_noop { stuff = "yay" }
> -      stuff.should == "yay"
> -    end
> -  end
>  end
> --
> 1.6.1
>
> --
> 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.
>
>



-- 
-----------------------------------------------------------
The power of accurate observation is
commonly called cynicism by those
who have not got it.  ~George Bernard Shaw
------------------------------------------------------------

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

Reply via email to