Puppet

I guess we need to change the behaviour here, but we need to provide backward compatibility, probably through some kind of recursion selection attribute:

file { "/etc/ssh": 
  source => "...",
  recurse => true,
  recurseselect => 
}

I don’t like that attribute name, but we need something, and at least for now, it should default to the current behaviour (probably with a deprecation warning).

luke wrote:

I guess we need to change the behaviour here, but we need to provide backward compatibility, probably through some kind of recursion selection attribute: […]

I don’t like that attribute name, but we need something, and at least for now, it should default to the current behaviour (probably with a deprecation warning).

Why not simply:

file { "/etc/ssh": 
  source => "...",
  recurse => ,
}

With true = both With false = no recurse

Why not simply: […] recurse => <remote|both|true|false>,

With true = both With false = no recurse

+1

masterzen wrote:

Why not simply: … recurse => <remote|both|true|false> …

With true = both With false = no recurse

This is a change in behaviour, but I’m not sure anyone’s actually using that behaviour. That is, you can say ‘recurse => 2’.

This means you’d either need to support multiple arguments to recurse (e.g., recurse => [remote, 2]) or change this behaviour. I expect it’s safe to change the behaviour, but….

luke wrote:

masterzen wrote:

Why not simply: … recurse => <remote|both|true|false> …

With true = both With false = no recurse

This is a change in behaviour, but I’m not sure anyone’s actually using that behaviour. That is, you can say ‘recurse => 2’.

Yes, I forgot this feature (which I used to workaround my original issue).

This means you’d either need to support multiple arguments to recurse (e.g., recurse => [remote, 2]) or change this behaviour. I expect it’s safe to change the behaviour, but….

I don’t really like the syntax you’re proposing. It is not really “typed”, and error prone (what happens if I write [2,remote] or [1,2]).

If we want to go this way, the following would IMHO be preferable:

... recurse => ,
    recurselimit => [0-9]+

Where:

recurse parameter:
 both => old behavior
 remote => new behavior (recurse on master, prune on local)
 true => same as both
 false => default (ie no-recurse)
 [0-9]+ => both, but limit recursion, give a warning to user that this syntax is deprecated and that they should move to recurselimit.

recurselimit parameter:
 [0-9]+ => recursion limit, usable with both (or true) and remote. If recurse=false it is ignored. If recurse is a number, then an error should be raised.

This way, we preserve the old behavior by default and we warn people that the syntax is deprecated.

Does it make sense?

Heh, I didn’t like the syntax I was proposing. :)

Your proposal makes perfect sense, although I would use ‘depth’ instead of ‘recurselimit’.

luke wrote:

Heh, I didn’t like the syntax I was proposing. :)

Your proposal makes perfect sense, although I would use ‘depth’ instead of ‘recurselimit’.

I’m all for it.

Since I’m working on it, it should be assigned to me :–)

Hi,

It took 5 months :–) but the patch is now available for master in my github repository (see tickets/1469): http://github.com/masterzen/puppet/tree/tickets/1469

Thanks,

Pushed in commit:“33d3624c91b236e237fe194d9df3d9fa324111c3” in branch master