Re: [Puppet-dev] [PATCH/puppet 1/1] (#4835) Fixes spurious changes when managing group membership on OS X

2011-04-12 Thread Daniel Pittman
On Tue, Apr 12, 2011 at 19:34, Nigel Kersten wrote: > On Tue, Apr 12, 2011 at 7:32 PM, Nigel Kersten wrote: >>  Unless you happened to have members added to a group in exactly the >> same order as they appear in your puppet manifests, Puppet considered >> the 'is' and 'should' values to be out of

Re: [Puppet-dev] [PATCH/puppet 1/1] (#4835) Fixes spurious changes when managing group membership on OS X

2011-04-12 Thread Nigel Kersten
On Tue, Apr 12, 2011 at 7:32 PM, Nigel Kersten wrote: >  Unless you happened to have members added to a group in exactly the > same order as they appear in your puppet manifests, Puppet considered > the 'is' and 'should' values to be out of sync due to ordering differences > between the arrays. >

[Puppet-dev] [PATCH/puppet 1/1] (#4835) Fixes spurious changes when managing group membership on OS X

2011-04-12 Thread Nigel Kersten
Unless you happened to have members added to a group in exactly the same order as they appear in your puppet manifests, Puppet considered the 'is' and 'should' values to be out of sync due to ordering differences between the arrays. The fix came in two small parts. a) Make the DirectoryServ

[Puppet-dev] [PATCH/puppet 1/1] (#6117) Add POST support to indirector requests

2011-04-12 Thread Nick Lewis
POST with a singular indirection is turned into a find in the indirector. When making a large find request from a REST terminus, POST is used, and for small requests, GET is used for backward compatibility. Paired-With: Jesse Wolfe Signed-off-by: Nick Lewis --- Local-branch: ticket/next/6117 l

Re: [Puppet-dev] [PATCH/facter 1/1] (#7039) Be more efficient when looking up single facts

2011-04-12 Thread Nigel Kersten
On Tue, Apr 12, 2011 at 2:36 PM, Jacob Helwig wrote: > Instead of always grabbing the full set of facts and only grabbing > those we're interested in, we attempt to look up every fact by the > "normal" means of "fact name == containing file name", and only fall > back to loading all of the facts i

[Puppet-dev] [PATCH/puppet 1/1] (#775) Don't require command when removing cron entries

2011-04-12 Thread Nick Lewis
Previously, if a cron entry had "ensure => absent" with no command specified and the crontab contained unmanaged entries, the entry would fail to be removed and every other cron entry in the catalog after it would be duplicated. Paired-With: Jesse Wolfe Signed-off-by: Nick Lewis --- Local-branch

Re: [Puppet-dev] [PATCH/puppet 1/1] (#2150) Add routes file for indirector

2011-04-12 Thread Luke Kanies
Sweet! I thought we'd never get this in, this is really awesome. On Apr 12, 2011, at 2:52 PM, Nick Lewis wrote: > Puppet[:route_file] is a YAML file specifying the termini to use for various > indirections, in the format: > > agent: > catalog: >terminus: rest >cache: yaml > master: >

[Puppet-dev] [PATCH/puppet 1/1] (#2150) Add routes file for indirector

2011-04-12 Thread Nick Lewis
Puppet[:route_file] is a YAML file specifying the termini to use for various indirections, in the format: agent: catalog: terminus: rest cache: yaml master: catalog: terminus: compiler cache: active_record This file is optional, and will override application defaults set in "s

[Puppet-dev] Re: [PATCH/facter 1/1] (#7039) Be more efficient when looking up single facts

2011-04-12 Thread Jacob Helwig
I should have changed the subject to RFC. This patch hasn't actually been merged into Facter. -- Jacob Helwig On Tue, 12 Apr 2011 14:36:43 -0700, Jacob Helwig wrote: > > Instead of always grabbing the full set of facts and only grabbing > those we're interested in, we attempt to look up every

[Puppet-dev] [PATCH/facter 1/1] (#7039) Be more efficient when looking up single facts

2011-04-12 Thread Jacob Helwig
Instead of always grabbing the full set of facts and only grabbing those we're interested in, we attempt to look up every fact by the "normal" means of "fact name == containing file name", and only fall back to loading all of the facts if this fails to find any of the facts we're looking for. Pair

[Puppet-dev] [PATCH/puppet 02/12] Refactor on the way to #5063 -- removing unused Scope#level

2011-04-12 Thread Nick Lewis
From: Markus Roberts This attribute is apparently no longer set or accessed. Signed-off-by: Nick Lewis --- Local-branch: ticket/next/5027 lib/puppet/parser/scope.rb|4 +--- spec/unit/parser/compiler_spec.rb |4 2 files changed, 1 insertions(+), 7 deletions(-) diff --git a

[Puppet-dev] [PATCH/puppet 09/12] Towards 5027 -- add options hash to lookupvar as with setvar

2011-04-12 Thread Nick Lewis
From: Markus Roberts This patch adds an options hash to lookupvar analogous to the one taken by setvar and uses it to pass in source location for error reporting. It also fixes the mechanism used by setvar (file was not being passed correctly), adds line and file information to errors in templat

[Puppet-dev] [PATCH/puppet 05/12] Refactor on the road to #5027 -- remove unused Scope#strinterp

2011-04-12 Thread Nick Lewis
From: Markus Roberts One of the uses of lookupvar was in the method Scope#strinterp; this method is no longer used (string interpolation is now handled by the parser (for the syntax) and AST nodes (for the semantics)) so this use of lookupvar can be excised, along with a fair amount of surroundin

[Puppet-dev] [PATCH/puppet 06/12] Step towards #5027 -- add Logging#deprication_warning facility

2011-04-12 Thread Nick Lewis
From: Markus Roberts This commit adds a method analogous to Puppet.warn which 1) only logs each message the first time it is received and 2) only logs the first 100 messages it receives. Messages are logged via warn. This could easily be made more flexible by making the hard limit and effective

[Puppet-dev] [PATCH/puppet 04/12] Refactor en route to #5027 -- remove usestring parameter from lookupvar

2011-04-12 Thread Nick Lewis
From: Markus Roberts The usestring parameter to lookupvar was objectionable for several reasons; first, it performed a function orthogonal to the main purpose of the method, second its default was the least common value, and third it was causing other code to work for reasons that were not obviou

[Puppet-dev] [PATCH/puppet 11/12] (#5027) Spell deprecation correctly

2011-04-12 Thread Nick Lewis
Paired-With: Jesse Wolfe Signed-off-by: Nick Lewis --- Local-branch: ticket/next/5027 lib/puppet/parser/scope.rb |2 +- lib/puppet/util/logging.rb | 10 +- spec/unit/util/logging_spec.rb | 12 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/l

[Puppet-dev] [PATCH/puppet 12/12] (#5027) Use Puppet#warning for deprecation_wanring instead of Kernel#warn

2011-04-12 Thread Nick Lewis
Using warning, the deprecation warnings will appear in logs and in color. Paired-With: Jesse Wolfe Signed-off-by: Nick Lewis --- Local-branch: ticket/next/5027 lib/puppet/util/logging.rb |2 +- spec/unit/util/logging_spec.rb |8 2 files changed, 5 insertions(+), 5 deletions

[Puppet-dev] [PATCH/puppet 07/12] Step towards #5027 -- scopes should know if they are dynamic

2011-04-12 Thread Nick Lewis
From: Markus Roberts The logic for distinguishing dynamic / static scopes was borrowed from Nick & Paul's patch, the main differences here being 1) calling it "dynamic" (true/ false) rather than "parent_relationship" (:inherited/:dynamic) 2) aligning the default so that it only needs to get set i

[Puppet-dev] [PATCH/puppet 10/12] Fix for #5027 -- generate a deprication warning for dynamic lookup

2011-04-12 Thread Nick Lewis
From: Markus Roberts This fix implements the same logic as Nick & Paul's patch in a different way. There aren't any tests yet and I'm still working out if I agree with the handling of some edge cases, so this should be considered premliminary. Signed-off-by: Nick Lewis --- Local-branch: ticket/

[Puppet-dev] [PATCH/puppet 08/12] Refactor for 5027 -- get rid of lookup_qualified_var

2011-04-12 Thread Nick Lewis
From: Markus Roberts Scope#lookup_qualified_var was a "magic bag" that took a qualified variable name apart and called back in to lookupvar with a new scope. This commit is a semantically neutral refactor that replaces it with a function to find the desired scope as a pure function (with error d

[Puppet-dev] [PATCH/puppet 03/12] Fix for #5063 -- explicitly scope internal variable lookups

2011-04-12 Thread Nick Lewis
From: Markus Roberts When we lookup a global variable / fact from code we should explicitly look in the global scope. Signed-off-by: Nick Lewis --- Local-branch: ticket/next/5027 lib/puppet/parser/functions/extlookup.rb |9 ++--- lib/puppet/parser/functions/fqdn_rand.rb |2

[Puppet-dev] [PATCH/puppet 01/12] Refactor prior to #5063 -- remove dead "topscope?" code

2011-04-12 Thread Nick Lewis
From: Markus Roberts The Scope#topscope? method was never called anywhere (including tests) and so far as I can tell was incorrect as the Scope#level is never being set. Signed-off-by: Nick Lewis --- Local-branch: ticket/next/5027 lib/puppet/parser/scope.rb |5 - 1 files changed, 0 ins

Re: [Puppet-dev] [PATCH/facter 1/1] (#7039) Pre-load all facts when requesting a single fact

2011-04-12 Thread Jacob Helwig
On Tue, 12 Apr 2011 12:27:02 -0700, Luke Kanies wrote: > > I think this is a good change, but it might be worth only doing this > if the fact returns nil without it. > > That is, if the fact is in an appropriately named file, it's fast, and > if not, we take the slower route of loading everything

Re: [Puppet-dev] [PATCH/facter 1/1] (#7039) Pre-load all facts when requesting a single fact

2011-04-12 Thread Luke Kanies
I think this is a good change, but it might be worth only doing this if the fact returns nil without it. That is, if the fact is in an appropriately named file, it's fast, and if not, we take the slower route of loading everything. -- http://puppetlabs.com/ | +1-615-594-8199 | @puppetmasterd On

Re: [Puppet-dev] [PATCH/puppet 1/1] (#3360) Add an allow_duplicate_certs option

2011-04-12 Thread Matt Robinson
On Tue, Apr 12, 2011 at 11:03 AM, Brice Figureau wrote: > Hi, > > On 12/04/11 19:50, Nick Lewis wrote: >> If this option is true, a certificate request with the same CN as an existing >> certificate will override the existing certificate when signed. With the >> option >> false, the new certifica

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Markus Roberts
I happen to do that quite frequently I'm afraid. > Sorry if it created some issues. I didn't expect (no pun intended) that > it wasn't working (and I'm positive this used to work). > > > Expecting a stubbed method never wasn't working, however expecting a > > stubbed method >= once behaved as expec

[Puppet-dev] [PATCH/facter 1/1] (#7039) Pre-load all facts when requesting a single fact

2011-04-12 Thread Jacob Helwig
Since multiple facts can be defined in a single file and we have no way of knowing which "additional" facts are defined in which files, we pre-load all facts when we're looking for specific one. Paired-with: Max Martin Signed-off-by: Jacob Helwig --- Local-branch: tickets/master/7039-multiple-f

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Brice Figureau
On 12/04/11 18:22, Jacob Helwig wrote: > On Tue, 12 Apr 2011 08:39:57 -0700, Markus Roberts wrote: >> >> I'm currrently looking at the specs for the ssh_authorized_key provider. There are two tests that give me a headache: it "should create the directory if it doesn't exist" do >>

Re: [Puppet-dev] [PATCH/puppet 1/1] (#3360) Add an allow_duplicate_certs option

2011-04-12 Thread Brice Figureau
Hi, On 12/04/11 19:50, Nick Lewis wrote: > If this option is true, a certificate request with the same CN as an existing > certificate will override the existing certificate when signed. With the > option > false, the new certificate request will be rejected. This option will default > to false.

[Puppet-dev] [PATCH/puppet 1/1] (#3360) Add an allow_duplicate_certs option

2011-04-12 Thread Nick Lewis
If this option is true, a certificate request with the same CN as an existing certificate will override the existing certificate when signed. With the option false, the new certificate request will be rejected. This option will default to false. Paired-With: Max Martin Signed-off-by: Nick Lewis

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Jacob Helwig
On Tue, 12 Apr 2011 09:28:44 -0700, Daniel Pittman wrote: > > On Tue, Apr 12, 2011 at 09:22, Jacob Helwig wrote: > > > Expecting a stubbed method never wasn't working, however expecting a > > stubbed method >= once behaved as expected.  Definitely an rspec bug. > > Meh. Have we identified the

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Daniel Pittman
On Tue, Apr 12, 2011 at 09:22, Jacob Helwig wrote: > Expecting a stubbed method never wasn't working, however expecting a > stubbed method >= once behaved as expected.  Definitely an rspec bug. Meh. Have we identified the versions that have this issue, and reported it upstream? Can we declare

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Jacob Helwig
On Tue, 12 Apr 2011 08:39:57 -0700, Markus Roberts wrote: > > I'm currrently looking at the specs for the ssh_authorized_key provider. > >> There are two tests that give me a headache: > >> > >>it "should create the directory if it doesn't exist" do > >> File.stubs(:exist?).with(@dir).ret

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Markus Roberts
S -- I'm currrently looking at the specs for the ssh_authorized_key provider. > There are two tests that give me a headache: > >it "should create the directory if it doesn't exist" do > File.stubs(:exist?).with(@dir).returns false > Dir.expects(:mkdir).with(@dir,0700) > @provide

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Markus Roberts
S -- I'm currrently looking at the specs for the ssh_authorized_key provider. >> There are two tests that give me a headache: >> >>it "should create the directory if it doesn't exist" do >> File.stubs(:exist?).with(@dir).returns false >> Dir.expects(:mkdir).with(@dir,0700) >> @

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Stefan Schulte
On Tue, Apr 12, 2011 at 05:38:54AM -0700, Luke Kanies wrote: > On Apr 12, 2011, at 3:24 AM, Stefan Schulte > wrote: > > > Hi, > > > > I'm currrently looking at the specs for the ssh_authorized_key provider. > > There are two tests that give me a headache: > > > >it "should create the director

Re: [Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Luke Kanies
On Apr 12, 2011, at 3:24 AM, Stefan Schulte wrote: > Hi, > > I'm currrently looking at the specs for the ssh_authorized_key provider. > There are two tests that give me a headache: > >it "should create the directory if it doesn't exist" do > File.stubs(:exist?).with(@dir).returns false >

[Puppet-dev] Wrong spec but passes anyways?!

2011-04-12 Thread Stefan Schulte
Hi, I'm currrently looking at the specs for the ssh_authorized_key provider. There are two tests that give me a headache: it "should create the directory if it doesn't exist" do File.stubs(:exist?).with(@dir).returns false Dir.expects(:mkdir).with(@dir,0700) @provider.flush

Re: [Puppet-dev] [PATCH/puppet 1/1] (#2331) Remove darwinports pkg provider, replace with rewritten macports provider

2011-04-12 Thread David Schmitt
On Mon, 11 Apr 2011 21:32:02 -0700, Daniel Pittman wrote: > *nod* My current thinking is that the right header looks roughly like: > > #!/usr/bin/env rspec > # ...and why couldn't they be consistent about where env lives? > require 'spec_helper' > > That should satisfy the direct e