Metaparam Warning

84 views
Skip to first unread message

Nan Liu

unread,
Jul 21, 2014, 1:37:24 PM7/21/14
to puppe...@googlegroups.com
I'm using puppet-system module and the usage of the schedule
metaparameter is generating a large amount of warnings similar to the
one below (about 30 lines):

Warning: schedule is a metaparam; this value will inherit to all
contained resources in the system::sysconfig::puppet definition

In this case, I think the behavior makes sense and I would like to
exclude this from appearing since the noise is masking more useful
warnings. I don't believe there's an option to disable warnings and in
this case I really want to just ignore it for this one module and not
globally disable it everywhere. I haven't found a good solution than
monkey patching puppet type:

alias_method :orig_warn_if_metaparam, :warn_if_metaparam

def warn_if_metaparam(param, default)
if self.name =~ /^system/ and param=='schedule'
return
else
orig_warn_if_metaparam(param, default)
end
end

Seems like there are other messages in puppet that people find annoying
than helpful:
https://github.com/boxen/puppet-boxen/blob/master/lib/puppet/provider/package/hax.rb

This seems pretty hideous, and I'm wondering if there are more sensible
way to solve this.

Thanks,

Nan

Andy Parker

unread,
Jul 21, 2014, 4:00:11 PM7/21/14
to puppe...@googlegroups.com
On Mon, Jul 21, 2014 at 10:37 AM, Nan Liu <nan...@gmail.com> wrote:
I'm using puppet-system module and the usage of the schedule metaparameter is generating a large amount of warnings similar to the one below (about 30 lines):

Warning: schedule is a metaparam; this value will inherit to all contained resources in the system::sysconfig::puppet definition


I'm actually surprised that is a warning and not a flat out error. The effect that it has is that now a user can't have a list of metaparams in their head when reading code and know when a parameter is *truly* one and and when it isn't.
 
In this case, I think the behavior makes sense and I would like to exclude this from appearing since the noise is masking more useful warnings. I don't believe there's an option to disable warnings and in this case I really want to just ignore it for this one module and not globally disable it everywhere. I haven't found a good solution than monkey patching puppet type:

alias_method :orig_warn_if_metaparam, :warn_if_metaparam

def warn_if_metaparam(param, default)
  if self.name =~ /^system/ and param=='schedule'
    return
  else
    orig_warn_if_metaparam(param, default)
  end
end

Seems like there are other messages in puppet that people find annoying than helpful:
https://github.com/boxen/puppet-boxen/blob/master/lib/puppet/provider/package/hax.rb

This seems pretty hideous, and I'm wondering if there are more sensible way to solve this.


We recently added "disable_warnings" and right now it can only deal with deprecations, but the design was such that it can affect other warnings as well. It would, however, be a global setting and I think it should stay as such.

I think what I'm trying to say is that, in this case, you really should rename that parameter for the sake of your users :)
 
Thanks,

Nan

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/53CD4FCF.1050201%40gmail.com.
For more options, visit https://groups.google.com/d/optout.



--
Andrew Parker
Freenode: zaphod42
Twitter: @aparker42
Software Developer

Join us at PuppetConf 2014September 22-24 in San Francisco
Register by May 30th to take advantage of the Early Adopter discount save $349!

Nan Liu

unread,
Jul 21, 2014, 4:32:47 PM7/21/14
to puppet-dev
On Mon, Jul 21, 2014 at 1:00 PM, Andy Parker <an...@puppetlabs.com> wrote:
On Mon, Jul 21, 2014 at 10:37 AM, Nan Liu <nan...@gmail.com> wrote:
I'm using puppet-system module and the usage of the schedule metaparameter is generating a large amount of warnings similar to the one below (about 30 lines):

Warning: schedule is a metaparam; this value will inherit to all contained resources in the system::sysconfig::puppet definition


I'm actually surprised that is a warning and not a flat out error. The effect that it has is that now a user can't have a list of metaparams in their head when reading code and know when a parameter is *truly* one and and when it isn't.

Not really my module: https://github.com/erwbgy/puppet-system, but I think the way module was written it should work because the value is passed as a default for create_resources. Here's an example:


In this case, I think the behavior makes sense and I would like to exclude this from appearing since the noise is masking more useful warnings. I don't believe there's an option to disable warnings and in this case I really want to just ignore it for this one module and not globally disable it everywhere. I haven't found a good solution than monkey patching puppet type:

alias_method :orig_warn_if_metaparam, :warn_if_metaparam

def warn_if_metaparam(param, default)
  if self.name =~ /^system/ and param=='schedule'
    return
  else
    orig_warn_if_metaparam(param, default)
  end
end

Seems like there are other messages in puppet that people find annoying than helpful:
https://github.com/boxen/puppet-boxen/blob/master/lib/puppet/provider/package/hax.rb

This seems pretty hideous, and I'm wondering if there are more sensible way to solve this.


We recently added "disable_warnings" and right now it can only deal with deprecations, but the design was such that it can affect other warnings as well. It would, however, be a global setting and I think it should stay as such.

I think what I'm trying to say is that, in this case, you really should rename that parameter for the sake of your users :)

The purpose of this parameter is to behave exactly the same as puppet metaparameter schedule:

It's rather unfortunate, but certainly doable to relabel all these parameter with another name on my fork. Similar to the discussion of collect resources realize + override, I wish the metaparameters weren't magically passed to the resources, and we could choose to use them and pass the behavior on as needed.

Nan

John Bollinger

unread,
Jul 21, 2014, 4:46:50 PM7/21/14
to puppe...@googlegroups.com


On Monday, July 21, 2014 3:00:11 PM UTC-5, Andy Parker wrote:
On Mon, Jul 21, 2014 at 10:37 AM, Nan Liu <nan...@gmail.com> wrote:
I'm using puppet-system module and the usage of the schedule metaparameter is generating a large amount of warnings similar to the one below (about 30 lines):

Warning: schedule is a metaparam; this value will inherit to all contained resources in the system::sysconfig::puppet definition


I'm actually surprised that is a warning and not a flat out error. The effect that it has is that now a user can't have a list of metaparams in their head when reading code and know when a parameter is *truly* one and and when it isn't.
 


I don't understand.  Surely 'schedule' is a documented metaparameter, and the behavior described in the warning is the normal behavior for metaparameters.  Why should this even be a warning, much less an error?  And how is any metaparameter not "truly" a metaparameter?  What am I missing here?


John

John Bollinger

unread,
Jul 21, 2014, 5:12:52 PM7/21/14
to puppe...@googlegroups.com


On Monday, July 21, 2014 3:32:47 PM UTC-5, Nan Liu wrote:


Oh, I see.  The class declares its own parameter 'schedule'.  Given that I'm not sure offhand what the semantics of that will -- or even should -- be, I have to agree with Andy that such code is nasty.  It at least warrants a warning.

Specifically, it's unclear whether the class's declared 'schedule' parameter refers to the metaparameter (so that it's a duplicate) vs. being an ordinary parameter that shadows the metaparameter.  If the former, then it's unclear whether the default value declared there will in fact be effective.

Furthermore, it's unclear what schedule will or should be assigned to the class if it is declared without an explicit schedule, but there is a schedule assigned to the container in which its first-evaluated declaration is found.  That is:

class site::wrapper {
  include 'system::packages'
}

class { 'site::wrapper': schedule => 'myschedule' }

What a mess!

With that said, if the construct in fact has a predictable, useful behavior (still not clear on that) then I think Nan's request for a way to silence the warnings is a reasonable one.


John

John Bollinger

unread,
Jul 21, 2014, 5:30:07 PM7/21/14
to puppe...@googlegroups.com


On Monday, July 21, 2014 3:32:47 PM UTC-5, Nan Liu wrote:
Similar to the discussion of collect resources realize + override, I wish the metaparameters weren't magically passed to the resources, and we could choose to use them and pass the behavior on as needed.



Sadly, it's not that easy.  Some metaparameters already aren't (or shouldn't) be forwarded, mainly 'alias'.  On the other hand, many others must be forwarded in order to be effective ('tag', 'noop', ...).  The relational metaparameters could technically swing either way, but the overall semantics need to maintain containment.

Schedule and loglevel are the only two that I think are ambiguous, but more loglevel than schedule.  I'm not sure how it makes sense for a resource to not inherit the schedule set on its container (if there is one), else it's container's assigned schedule isn't fully honored.  On the other hand, it does make sense for a resource to have a different (lower) loglevel than its container, especially if the container's is interpreted as an upper bound rather than an exact log level for all messages.


John

 

Nan Liu

unread,
Jul 21, 2014, 6:29:19 PM7/21/14
to puppet-dev

The warning message seems to be a sign the behavior is not well defined which leaves much to be desired. I was hoping there was some path forward:

1. Not supported, don't reuse metaparameter as class parameter. Deprecate it, and enforce with failure.
2. Supported, metaparameter as class parameter pass behavior to contained resources. In this scenario, I would argue metaparameter should be allowed without being specified as a class parameter explicitly (see ticket).
https://tickets.puppetlabs.com/browse/PUP-2556
3. Supported, metaparameter as class parameter behavior is configured by user, support is enabled by specifying the parameter.

I'm not sure what are the other solutions beyond the three above, but removing the ambiguity we have now would be an improvement.

Nan 

David Schmitt

unread,
Jul 22, 2014, 2:32:56 AM7/22/14
to puppe...@googlegroups.com
On 2014-07-22 00:28, Nan Liu wrote:
> On Mon, Jul 21, 2014 at 2:30 PM, John Bollinger
> <john.bo...@stjude.org <mailto:john.bo...@stjude.org>> wrote:
>
>
>
> On Monday, July 21, 2014 3:32:47 PM UTC-5, Nan Liu wrote:
>
> Similar to the discussion of collect resources realize +
> override, I wish the metaparameters weren't magically passed to
> the resources, and we could choose to use them and pass the
> behavior on as needed.
>
>
>
> Sadly, it's not that easy. Some metaparameters already aren't (or
> shouldn't) be forwarded, mainly 'alias'. On the other hand, many
> others /must/ be forwarded in order to be effective ('tag', 'noop',
> ...). The relational metaparameters could technically swing either
> way, but the overall semantics need to maintain containment.
>
> Schedule and loglevel are the only two that I think are ambiguous,
> but more loglevel than schedule. I'm not sure how it makes sense
> for a resource to not inherit the schedule set on its container (if
> there is one), else it's container's assigned schedule isn't fully
> honored. On the other hand, it does make sense for a resource to
> have a different (lower) loglevel than its container, especially if
> the container's is interpreted as an upper bound rather than an
> exact log level for all messages.
>
>
> The warning message seems to be a sign the behavior is not well defined
> which leaves much to be desired. I was hoping there was some path forward:
>
> 1. Not supported, don't reuse metaparameter as class parameter.
> Deprecate it, and enforce with failure.
> 2. Supported, metaparameter as class parameter pass behavior to
> contained resources. In this scenario, I would argue metaparameter
> should be allowed without being specified as a class parameter
> explicitly (see ticket).
> https://tickets.puppetlabs.com/browse/PUP-2556
> 3. Supported, metaparameter as class parameter behavior is configured by
> user, support is enabled by specifying the parameter.
>
> I'm not sure what are the other solutions beyond the three above, but
> removing the ambiguity we have now would be an improvement.

I like #3, but despair when thinking about the easy failure modes
(accidentally overwriting a metaparam with something completely different).

Maybe the way forward lies in treating the different kinds of metaparams
- as John has enumerated - also differently:

* "must be forwarded in order to be effective ('tag', 'noop', ...)":
These should not be overrideable. If someone further up has
set noop, nobody should be able to escape that (what about
collections in this dynamic scope? *shudder*)
Trying to have a define or class with such a parameter should be an
error.

* "aren't (or shouldn't) be forwarded, mainly 'alias'":
Either it's allowable to override those, then remove the warning.
OR it's not allowed, then fail with an error.
Personally, I don't think 'alias' should be overrideable, but I have
no argument for that.

* "relational metaparameters could technically swing either way, but
the overall semantics need to maintain containment": This could
definitely be overridable, the user should be aware that he's
expected to maintain the semantics.



Regards, David
--
* Always looking for people I can help with awesome projects *
G+: https://plus.google.com/+DavidSchmitt
Blog: http://club.black.co.at/log/
LinkedIn: http://at.linkedin.com/in/davidschmitt

Henrik Lindberg

unread,
Jul 22, 2014, 8:48:19 AM7/22/14
to puppe...@googlegroups.com
To me puppet's meta parameters, are just like other attributes of an
object and have traits; readable, writable, virtual, derived,
multiplicity, data-type, private, final, volatile.

Without having a way of specifying the contract for a meta parameter how
does subclasses know what they should implement? How can this be
tested/enforced?

Some of the listed traits cannot be implemented in the Puppet Language;
it is not possible to pass on a call to a "super parameter
implementation", we cannot do before/after/around (we only have
instead-of), we have very small means of specifying logic to run late in
the lazy evaluation, all of the information required to implement some
meta parameter behaviors is not available until after we stopped
evaluating the puppet logic.

Currently the only thing we can do with some sort of safety is to make
them final. (i.e. option number 1).

BTW; the "meta-like" parameters $name and $title have been blocked in
the future parser (they raise an error), so you can say we made them final.

Later, it may be possible to use the Puppet Type system, to declare that
a parameter is of a type derived from a MetaParameter type. This because
it will be possible to define behavior for types (i.e. data types / not
resource types). At that point, it is possible to have a richer set of
traits available in the Puppet Language and we can remove the final
restrictions on those meta parameters where it makes sense (i.e. option
#3 with David S's addition).

The above means, that in order for a Class to re-implement a
meta-parameter, that parameter needs to refer to a defined type, that in
turn declares that it is derived from the meta-data-type in question.

type MySchedule inherits MetaParameter[Schedule] {
# The stuff that needs to be done for the derived schedule
}

class myclass(MySchedule $schedule) {
}

Not typing it to be a kind of Schedule breaks the contract, and that
would be an error.

- henrik

--

Visit my Blog "Puppet on the Edge"
http://puppet-on-the-edge.blogspot.se/

Reply all
Reply to author
Forward
0 new messages