Do you rely on 'param=>undef' being equal to '(nothing)'?

[Eric Sorenson]

Hi, there's an issue that came up recently in the 3.0RCs -- Big thanks to Erik Dalén for reporting it in #16221 -- that involves a behaviour change to part of the DSL. In a nutshell, this code:

define foobar ($param='Hello world') {
notice($param)
}
foobar { 'test': param => undef }

in 2.7, causes 'Hello world' in the notice. In 3.x, it's nothing. As I said in the bug, this seems more correct to me -- I've overriden the default with an explicit 'undef', taking off the default. The same thing goes for invoking parameterised classes with undef arguments, which is perhaps more ambiguous (example from matthaus):

class toplevel (
$maybe = false,
$optional = undef ) {
if ($maybe) {
class { toplevel::secondlevel: optional => undef }
}
}

In order to make use of the default for the `optional` parameter in toplevel::secondlevel, you'd now need to either test in `toplevel` whether `$optional` was passed into it, or have toplevel::secondlevel use an `$optional_real` value inside it, similar to what's commonly done to append to defaults that are array values.

The closest thing to documentation around this suggests the new behaviour is what's intended <http://docs.puppetlabs.com/puppet/2.7/reference/lang_classes.html#overriding-resource-attributes>:

You can remove an attribute’s previous value without setting a new one by overriding it with the special value undef:

class base::freebsd inherits base::unix {
File['/etc/passwd'] {
group => undef,
}
}

So, I'm trying to determine whether this is a widespread pattern or an edge-case. Do you expect 'param=>undef' to be the same as not specifying param at all, or for the receiver to "see" the undef?

Eric Sorenson - eric.s...@puppetlabs.com
PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12

[Aaron Grewell]

I'm using the current behavior in inherited classes to unset parameters set by the parent class.  If that no longer works it will definitely impact my code.

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppe...@googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

[Stefan Schulte]

On Fri, Sep 14, 2012 at 11:31:08AM -0700, Eric Sorenson wrote:
> Hi, there's an issue that came up recently in the 3.0RCs -- Big thanks to Erik Dal�n for reporting it in #16221 -- that involves a behaviour change to part of the DSL. In a nutshell, this code:
>
[..]>

> class toplevel (
> $maybe = false,
> $optional = undef ) {
> if ($maybe) {
> class { toplevel::secondlevel: optional => undef }
> }
> }
>
> In order to make use of the default for the `optional` parameter in toplevel::secondlevel, you'd now need to either test in `toplevel` whether `$optional` was passed into it, or have toplevel::secondlevel use an `$optional_real` value inside it, similar to what's commonly done to append to defaults that are array values.
>

[...]

>
> So, I'm trying to determine whether this is a widespread pattern or an edge-case. Do you expect 'param=>undef' to be the same as not specifying param at all, or for the receiver to "see" the undef?
>
> Eric Sorenson - eric.s...@puppetlabs.com
> PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12
>

I use this a lot to be able to have an optional parameter in a parent
class that is passed to an included class and the included class
determines the default value. Like:

class basic($puppet_cron = undef) {
class { 'puppet::client':
cron => $puppet_cron,
}
}

-Stefan

[Trevor Vaughan]

I also use the undef to un-set parameters in various code segments. If
this is made real, it will impact my code.

Thanks,

Trevor

On Fri, Sep 14, 2012 at 6:39 PM, Stefan Schulte
<stefan....@taunusstein.net> wrote:
> On Fri, Sep 14, 2012 at 11:31:08AM -0700, Eric Sorenson wrote:

>> Hi, there's an issue that came up recently in the 3.0RCs -- Big thanks to Erik Dalén for reporting it in #16221 -- that involves a behaviour change to part of the DSL. In a nutshell, this code:

>>
> [..]>
>> class toplevel (
>> $maybe = false,
>> $optional = undef ) {
>> if ($maybe) {
>> class { toplevel::secondlevel: optional => undef }
>> }
>> }
>>
>> In order to make use of the default for the `optional` parameter in toplevel::secondlevel, you'd now need to either test in `toplevel` whether `$optional` was passed into it, or have toplevel::secondlevel use an `$optional_real` value inside it, similar to what's commonly done to append to defaults that are array values.
>>
> [...]
>>
>> So, I'm trying to determine whether this is a widespread pattern or an edge-case. Do you expect 'param=>undef' to be the same as not specifying param at all, or for the receiver to "see" the undef?
>>
>> Eric Sorenson - eric.s...@puppetlabs.com
>> PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12
>>
>
> I use this a lot to be able to have an optional parameter in a parent
> class that is passed to an included class and the included class
> determines the default value. Like:
>
> class basic($puppet_cron = undef) {
> class { 'puppet::client':
> cron => $puppet_cron,
> }
> }
>
> -Stefan
>

> --
> You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
>

--
Trevor Vaughan
Vice President, Onyx Point, Inc
(410) 541-6699
tvau...@onyxpoint.com

-- This account not approved for unencrypted proprietary information --

[jcbollinger]

I expect param => undef to be an explicit expression of 'param' not being specified by the current declaration, or as an override, to express the idea of the overridden resource as if 'param' had not been specified.  The latter is essential.  The former follows for consistency, because

foobar { 'example':
  param => undef
}

should not express a different resource state than does

Foobar['example'] {
  param => undef
}

Moreover, my gut feeling is that having the undef bind to the actual parameter instead of being declaration metasyntax implies a need for users to have more knowledge of the details of the definition's (or class's) implementation than is reasonable.  Though users may indeed have such information in some cases, that still doesn't sit well with me.


John

[Eric Sorenson]

Aaron -- could you distill this down to a code sample? Unless I'm misunderstanding, it sounds like your case is slightly different to the ones I posted. Thanks.

[Daniel Pittman]

On Mon, Sep 17, 2012 at 11:58 AM, Eric Sorenson
<eric.s...@puppetlabs.com> wrote:
> Aaron -- could you distill this down to a code sample? Unless I'm
> misunderstanding, it sounds like your case is slightly different to the ones
> I posted. Thanks.

I recognise the case; this is a different use and, as far as I
understand, shouldn't be changed.

(It is also consistent with how the *new* behaviour of undef vs
parameter defaults works. :)

> "Puppet Users" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/puppet-users/-/KtqHUAhcelcJ.
> To post to this group, send email to puppet...@googlegroups.com.

> To unsubscribe from this group, send email to

> puppet-users...@googlegroups.com.

> For more options, visit this group at

> http://groups.google.com/group/puppet-users?hl=en.



--
Daniel Pittman
⎋ Puppet Labs Developer – http://puppetlabs.com
♲ Made with 100 percent post-consumer electrons

[Eric Sorenson]

On Friday, September 14, 2012 3:36:20 PM UTC-7, Stefan Schulte wrote:

I use this a lot to be able to have an optional parameter in a parent
class that is passed to an included class and the included class
determines the default value. Like:

    class basic($puppet_cron = undef) {
      class { 'puppet::client':
        cron => $puppet_cron,
      }
    }

Stefan - This is exactly the case I'm concerned about.

The simplest way to preserve this pattern would be putting the default value for cron in the `basic` typedef, where you now have `undef`.

class basic($puppet_cron = "some_default") {

   class { 'puppet::client': 

      cron => $puppet_cron,

  }

}

The advantage is that if you want to really revert to the default for the `cron` parameter, you can actually invoke it with undef, just like a regular resource.

The bad side is that you now have to move your defaults to the calling class, or worse, duplicate them.

What do you think?

-=Eric

[Eric Sorenson]

On Saturday, September 15, 2012 5:06:52 PM UTC-7, Trevor Vaughan wrote:

I also use the undef to un-set parameters in various code segments. If
this is made real, it will impact my code.

Trevor, if I understand correctly I think using undef to un-set parameters won't change. If anything it will be preserved in cases (i.e. into defined types) where it currently gets silently dropped. Do you have a little code sample?

-=Eric

[Stefan Schulte]

Eric -

In my case a class that is included in a wrapper class is not supposed
to be included directly at node level. So I could move the default
values in the wrapper class.

This would also make documenting the wrapper class cleaner (I dont have
to write "param foo determines bar. For the default value check the
documentation of class X"). On the other hand I am not able to include
the class directly anymore without specifying all paramters explicitly
(as I said I currently don't do that, but others might). Duplicating the
default value seems like a bad thing to do, especially when they get out
of sync. And I am not sure if I like the fact that the knowledge "what
is a sane default value for this particular parameter" is shifting out
of the actual class.

-Stefan

[Trevor Vaughan]

After reading through the follow up messages, I think that the
proposed change won't affect me at all.

Thanks for the follow up.

Trevor

> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.

> To view this discussion on the web visit

> https://groups.google.com/d/msg/puppet-dev/-/LzxOfpLRUTUJ.

[Eric Sorenson]

Friends -- 

As of 30da528e9c8ac345d1020d1cbeee02598174a3c5 we put this intentionally back to the 2.7 behaviour. Because both

(a) the change crept in as a side-effect of other code changes, not as a deliberate language design choice

 -and-

(b) we are not providing a simple (sigil or keyword) workaround for people to preserve the behaviour they've written around.

are true here, I'm convinced it's not a good change. After this commit, the sample code in Erik's update 1 works as expected, as do the other cases I'm able to test.

Thanks for your feedback.

Eric Sorenson - eric.s...@puppetlabs.com 
PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12