Re: [Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread James Turnbull
Luke Kanies wrote: On May 16, 2011, at 12:38 PM, James Turnbull wrote: Randall Hansen wrote: On Mon, May 16, 2011 at 10:50 AM, Nick Lewis wrote: IIRC the intent was for external faces that were being developed by a user not to affect the functionality of core Puppet. I agree with this. I

Re: [Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread Luke Kanies
On May 16, 2011, at 10:34 AM, Matt Robinson wrote: > Contrary to the current code and ticket #7314 I think syntax errors in > Faces should raise errors immediately instead of just warning in the > log. I could be convinced otherwise, but the discussion with the > original coder of the current beh

Re: [Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread Luke Kanies
On May 16, 2011, at 12:38 PM, James Turnbull wrote: > Randall Hansen wrote: >> On Mon, May 16, 2011 at 10:50 AM, Nick Lewis wrote: >> >>> IIRC the intent was for >>> external faces that were being developed by a user not to affect the >>> functionality of core Puppet. >> >> I agree with this.

Re: [Puppet-dev] [PATCH/puppet 1/2] (#7474) Add support for web request type and provider

2011-05-16 Thread Luke Kanies
Thanks for the patches. How do you expect to use this type? Can you provide some example use cases? I've got a comment below, too. On May 16, 2011, at 8:22 PM, Mo Morsi wrote: > Adds a new resource type to puppet for web requests > and implements a provider of that type using the > ruby curl i

[Puppet-dev] [PATCH/puppet 1/2] (#7474) Add support for web request type and provider

2011-05-16 Thread Mo Morsi
Adds a new resource type to puppet for web requests and implements a provider of that type using the ruby curl interface provided by the 'curb' rubygem Signed-off-by: Mo Morsi --- Local-branch: feature/master/7474 lib/puppet/provider/web_request/curl.rb | 128 +++ li

[Puppet-dev] [PATCH/puppet 2/2] (#7474) adds test for web request type and provider

2011-05-16 Thread Mo Morsi
Signed-off-by: Mo Morsi --- Local-branch: feature/master/7474 spec/unit/provider/web/curl_spec.rb | 32 spec/unit/type/web_request_spec.rb | 29 + 2 files changed, 61 insertions(+), 0 deletions(-) create mode 100644 spec/unit/pro

[Puppet-dev] feature 7474 against master

2011-05-16 Thread Mo Morsi
This patchset implements a new resource type representing web resources/requests which can be included in a puppet scipt. A default provider of this type is also included using the curl interface for ruby as provided by the 'curb' gem The second patch implements a few spec cases for the new type

[Puppet-dev] [PATCH/puppet 4/4] (#7291) Fix issues with instance_methods in Ruby 1.9

2011-05-16 Thread Matt Robinson
instance_methods in Ruby 1.8.7 returns an array of strings, but returns an array of symbols in 1.9.2. This manifested itself when running the tests because in 1.9.2 we were trying to call sub on a sybmol. The original proposed solution was to monkey patch symbols to have a sub method, but this di

[Puppet-dev] [PATCH/puppet 1/1] (#7468) Stub spec that tries to connect to pypi.python.org

2011-05-16 Thread Matt Robinson
I noticed a test failure when I ran the specs without an internet connection. Specs should never need an internet connection to pass. Reviewed-by: Max Martin Signed-off-by: Matt Robinson --- Local-branch: ticket/2.7.x/7468-stub_xmlrpc_internet_call spec/unit/provider/package/pip_spec.rb | 44

[Puppet-dev] [PATCH/puppet 1/4] (#7291) The 'script' version of actions needs options argument

2011-05-16 Thread Matt Robinson
Ruby 1.9 is stricter about arity for for arguments passed as a block. A test case just hadn't been updated to take this into account for Face scripts (a simpler form of action), which isn't surprising since script isn't used anywhere in the code, which makes it a prime candidate for removal as far

[Puppet-dev] [PATCH/puppet 2/4] (#7291) Fix Ruby 1.9 face failures

2011-05-16 Thread Matt Robinson
The certificate face wasn't being loaded, but it wasn't clear from the test failure why: lib/puppet/interface.rb:61:in `[]': Could not find Puppet Face :certificate (Puppet::Error) The problem is that when the certificate face is required you get: SyntaxError Exception: /Users/ma

[Puppet-dev] [PATCH/puppet 3/4] (#7291) Fixed ascii problem with Ruby 1.9.2

2011-05-16 Thread Matt Robinson
As with the previous commit, there was a problem loading a face because Ruby 1.9.2 doesn't like using non-standard ascii characters without declaring the encoding at the top of the file. SyntaxError Exception: /Users/matthewrobinson/work/puppet/lib/puppet/face/resource.rb:10: invalid multibyte ch

Re: [Puppet-dev] [PATCH/mcollective 1/1] 7532 - Facts name validation regex does not allow dots

2011-05-16 Thread R.I.Pienaar
- Original Message - > On Mon, May 16, 2011 at 14:20, R.I.Pienaar wrote: > > - Original Message - > >> On Mon, May 16, 2011 at 14:02, R.I.Pienaar wrote: > >> > Allow . in fact names > >> > >> This doesn't seem sufficient to me: facter, at least, defines the > >> fact name as "an

Re: [Puppet-dev] [PATCH/mcollective 1/1] 7532 - Facts name validation regex does not allow dots

2011-05-16 Thread Daniel Pittman
On Mon, May 16, 2011 at 14:20, R.I.Pienaar wrote: > - Original Message - >> On Mon, May 16, 2011 at 14:02, R.I.Pienaar wrote: >> > Allow . in fact names >> >> This doesn't seem sufficient to me: facter, at least, defines the >> fact name as "any arbitrary string, after String#downcase", w

Re: [Puppet-dev] [PATCH/puppet 1/1] (#7291) Monkeypatch Symbol with #sub method for ruby 1.9

2011-05-16 Thread Matt Robinson
This initially seemed like a good idea to me, but after playing with it there's some issues. The real problem isn't whether there's a sub method on Symbol, but that instance_methods used to return Strings in Ruby 1.8.7 and now returns Symbols in 1.9. So by monkey patching symbols to convert to a

Re: [Puppet-dev] [PATCH/mcollective 1/1] 7532 - Facts name validation regex does not allow dots

2011-05-16 Thread R.I.Pienaar
- Original Message - > On Mon, May 16, 2011 at 14:02, R.I.Pienaar wrote: > > Allow . in fact names > > This doesn't seem sufficient to me: facter, at least, defines the > fact name as "any arbitrary string, after String#downcase", which means > that this is still substantially more rest

Re: [Puppet-dev] [PATCH/mcollective 1/1] 7532 - Facts name validation regex does not allow dots

2011-05-16 Thread Daniel Pittman
On Mon, May 16, 2011 at 14:02, R.I.Pienaar wrote: > Allow . in fact names This doesn't seem sufficient to me: facter, at least, defines the fact name as "any arbitrary string, after String#downcase", which means that this is still substantially more restrictive than one of the underlying librarie

[Puppet-dev] [PATCH/mcollective 1/1] 7532 - Facts name validation regex does not allow dots

2011-05-16 Thread R.I.Pienaar
Allow . in fact names Signed-off-by: R.I.Pienaar --- Local-branch: bug/master/7532 plugins/mcollective/agent/rpcutil.ddl |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/plugins/mcollective/agent/rpcutil.ddl b/plugins/mcollective/agent/rpcutil.ddl index 9f70094..d478a4d

[Puppet-dev] [PATCH/mcollective 1/1] 7448 - Init script fails to start on RHEL4

2011-05-16 Thread R.I.Pienaar
Fix RH4 compatability Signed-off-by: R.I.Pienaar --- Local-branch: bug/master/7448 mcollective.init-rh |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/mcollective.init-rh b/mcollective.init-rh index 7a31dea..09d0dc6 100755 --- a/mcollective.init-rh +++ b/mcollective.ini

Re: [Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread James Turnbull
Randall Hansen wrote: On Mon, May 16, 2011 at 10:50 AM, Nick Lewis wrote: IIRC the intent was for external faces that were being developed by a user not to affect the functionality of core Puppet. I agree with this. If a Puppet user installs one bad Face it shouldn't prevent him working wit

Re: [Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread Daniel Pittman
On Mon, May 16, 2011 at 10:34, Matt Robinson wrote: > Contrary to the current code and ticket #7314 I think syntax errors in > Faces should raise errors immediately instead of just warning in the > log.  I could be convinced otherwise, but the discussion with the > original coder of the current b

Re: [Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread Randall Hansen
On Mon, May 16, 2011 at 10:50 AM, Nick Lewis wrote: > IIRC the intent was for > external faces that were being developed by a user not to affect the > functionality of core Puppet. I agree with this. If a Puppet user installs one bad Face it shouldn't prevent him working with all other Faces.

[Puppet-dev] [PATCH/puppet 2/2] (#7259) Remove ActiveRecord requirement from indirector face spec

2011-05-16 Thread Jacob Helwig
"should be able to return a list of terminuses for a given indirection" was calling Puppet::Indirector::Face.terminus_classes, which is just a thin wrapper around Puppet::Indirector::Terminus.terminus_classes, which would attempt to load all Terminus classes. This would cause problems if not all o

[Puppet-dev] [PATCH/puppet 1/2] (#7259) Do not try to load all Terminus classes when configuring the Indirector

2011-05-16 Thread Jacob Helwig
When configuring the Indirector routes, we should only try loading the Terminus classes that are referenced by the configuration. Previously, we were loading all Terminus classes, which would cause errors if we didn't have all of the prerequisites for all of them, even if the ones with missing pre

Re: [Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread Nick Lewis
On Monday, May 16, 2011 at 10:34 AM, Matt Robinson wrote: Contrary to the current code and ticket #7314 I think syntax errors in > Faces should raise errors immediately instead of just warning in the > log. I could be convinced otherwise, but the discussion with the > original coder of the current

Re: [Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread Jacob Helwig
On Mon, 16 May 2011 10:34:45 -0700, Matt Robinson wrote: > > Contrary to the current code and ticket #7314 I think syntax errors in > Faces should raise errors immediately instead of just warning in the > log. I could be convinced otherwise, but the discussion with the > original coder of the cur

[Puppet-dev] Should fail early when faces have syntax errors

2011-05-16 Thread Matt Robinson
Contrary to the current code and ticket #7314 I think syntax errors in Faces should raise errors immediately instead of just warning in the log. I could be convinced otherwise, but the discussion with the original coder of the current behavior and the aforementioned ticket haven't done so. Genera