Yeah, Ken also made a suggestion when we skip files to use
Puppet.debug instead so it's less verbose. I'm going to make some
changes and send a new patch.

Thanks,

Nan

On Fri, Jul 15, 2011 at 4:54 PM, Jacob Helwig <[email protected]> wrote:
> At first glance this doesn't look unreasonable, but the commit message could 
> definitely use some work.  Why is this a good change?  What problem does it 
> solve?  I'd want to see the answers to questions like that in the commit 
> message.
>
> --
> Please excuse my brevity; sent from my mobile device.
>
> On Jul 15, 2011, at 4:36 PM, Nan Liu <[email protected]> wrote:
>
>> Changed pluginsync so it does not load any non ruby files.
>>
>> Thanks,
>>
>> Nan
>> ---
>> lib/puppet/configurer/plugin_handler.rb     |    8 ++++++--
>> spec/unit/configurer/plugin_handler_spec.rb |   23 +++++++++++++++--------
>> 2 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/puppet/configurer/plugin_handler.rb 
>> b/lib/puppet/configurer/plugin_handler.rb
>> index ae088f2..3806b58 100644
>> --- a/lib/puppet/configurer/plugin_handler.rb
>> +++ b/lib/puppet/configurer/plugin_handler.rb
>> @@ -24,8 +24,12 @@ module Puppet::Configurer::PluginHandler
>>     return if FileTest.directory?(file)
>>
>>     begin
>> -      Puppet.info "Loading downloaded plugin #{file}"
>> -      load file
>> +      if file =~ /.rb$/
>> +        Puppet.info "Loading downloaded plugin #{file}"
>> +        load file
>> +      else
>> +        Puppet.info "Skipping downloaded plugin #{file}"
>> +      end
>>     rescue Exception => detail
>>       Puppet.err "Could not load downloaded file #{file}: #{detail}"
>>     end
>> diff --git a/spec/unit/configurer/plugin_handler_spec.rb 
>> b/spec/unit/configurer/plugin_handler_spec.rb
>> index 7d99960..0fb7183 100755
>> --- a/spec/unit/configurer/plugin_handler_spec.rb
>> +++ b/spec/unit/configurer/plugin_handler_spec.rb
>> @@ -75,18 +75,25 @@ describe Puppet::Configurer::PluginHandler do
>>     @pluginhandler.download_plugins
>>   end
>>
>> -  it "should load plugins when asked to do so" do
>> +  it "should load ruby plugins when asked to do so" do
>>     FileTest.stubs(:exist?).returns true
>> -    @pluginhandler.expects(:load).with("foo")
>> +    @pluginhandler.expects(:load).with("foo.rb")
>> +
>> +    @pluginhandler.load_plugin("foo.rb")
>> +  end
>> +
>> +  it "should skip non-ruby plugins when asked to do so" do
>> +    FileTest.stubs(:exist?).returns true
>> +    @pluginhandler.expects(:load).never
>>
>>     @pluginhandler.load_plugin("foo")
>>   end
>>
>>   it "should not try to load files that don't exist" do
>> -    FileTest.expects(:exist?).with("foo").returns false
>> +    FileTest.expects(:exist?).with("foo.rb").returns false
>>     @pluginhandler.expects(:load).never
>>
>> -    @pluginhandler.load_plugin("foo")
>> +    @pluginhandler.load_plugin("foo.rb")
>>   end
>>
>>   it "should not try to load directories" do
>> @@ -99,17 +106,17 @@ describe Puppet::Configurer::PluginHandler do
>>
>>   it "should warn but not fail if loading a file raises an exception" do
>>     FileTest.stubs(:exist?).returns true
>> -    @pluginhandler.expects(:load).with("foo").raises "eh"
>> +    @pluginhandler.expects(:load).with("foo.rb").raises "eh"
>>
>>     Puppet.expects(:err)
>> -    @pluginhandler.load_plugin("foo")
>> +    @pluginhandler.load_plugin("foo.rb")
>>   end
>>
>>   it "should warn but not fail if loading a file raises a LoadError" do
>>     FileTest.stubs(:exist?).returns true
>> -    @pluginhandler.expects(:load).with("foo").raises LoadError.new("eh")
>> +    @pluginhandler.expects(:load).with("foo.rb").raises LoadError.new("eh")
>>
>>     Puppet.expects(:err)
>> -    @pluginhandler.load_plugin("foo")
>> +    @pluginhandler.load_plugin("foo.rb")
>>   end
>> end
>> --
>> 1.7.5.3
>>
>> --
>> 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.
>>
>



-- 
==========================================
Join us in PDX for PuppetConf: http://bit.ly/puppetconfsig

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