How strict do you want puppet to be?

86 views
Skip to first unread message

Henrik Lindberg

unread,
Feb 22, 2016, 7:50:07 PM2/22/16
to puppe...@googlegroups.com
Hi, I am thinking ahead a bit regarding puppet 5 and how we should deal
with all the requests for features that require deprecations. (There are
some related things like requests for additional validation and warnings
that are different from deprecations).

In the past we merrily started issuing deprecation warnings, but the
community pretty unanimously said "stop doing that" we cannot deal with
all of those warnings. Since then we then pretty much stopped doing
deprecation warnings.

There has also been a long standing wish for a "strict mode" in puppet,
that like a harsh teacher would point out every itty-bitty problem.

So - what should we do?

In PUP-5889 I have described an idea. This is the text from that ticket
as it stands right now.

PUP-5889
---------------------------------------------------------------------
Add a flag --strict to puppet settings. When in effect this will turn on
--strict_variables, and will also enable other "helpful" but undesirable
behavior. (Each such behavior to be defined in a separate ticket).

The semantics of this flag should be:

* '--strict=ignore'; no strictness checks are to be performed, nothing
is reported.

* '--strict=warn'; strictness checks are performed, they are reported as
warnings, individually configurable warnings follow their own setting
(i.e. if they are added to disabled_warnings).

* '--strict=errors'; strictness checks are performed, they are reported
as errors and stop the execution. Further configuration to error
individually is not supported.

When we add this we promise to not change the set of things that lead to
warning/error in .z releases, but we reserve the right to do so for .x
releases. The idea being is that you can safely accept updates for .z
without having to do anything. For .x releases you may need to step back
to '--strict=warning' and then correct the problem before going back to
'--strict=error'.

This scheme should cater to those that are pedantic about following best
practices and not using deprecated features while those that only care
at major version boundaries can do so in peace without being bothered
with lots of deprecation warnings.
----------------------------------------------------------------------

What do you think about this idea? Control all strictness and
deprecation warnings/errors with one flag, and handle individual ones
(where applicable) by disabling those checks.

The benefit for us developing puppet is that we can introduce the new
behavior much sooner and we do not need to add flags for each and every
kind of validation/deprecation. This means we are more likely to improve
things as we are not holding off until the very last release in a major
series (and where inevitably some tickets will not make it).

Ironically, if this feature is liked it will make it into 4.5.0 which
may be the last in the 4.x series, but no decision has been made yet.

- henrik

--

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

Ben Ford

unread,
Feb 23, 2016, 5:31:18 PM2/23/16
to puppe...@googlegroups.com
Would it be possible in this scheme to mark strict mode per class? I could mark my own code as being strict and therefore get compile time failures when I make a typo myself, but wouldn't have to enforce that on all third party code.


--
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+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/naga6d%245r3%241%40ger.gmane.org.
For more options, visit https://groups.google.com/d/optout.



--
Ben Ford | Training Solutions Engineer 
Puppet Labs, Inc.
308 SW 2nd Ave
Fifth floor
Portland, OR 97209

509.592.7291
ben....@puppetlabs.com

Walter Heck

unread,
Feb 23, 2016, 6:22:30 PM2/23/16
to Puppet Developers
On Tuesday, February 23, 2016 at 11:31:18 PM UTC+1, Ben Ford wrote:
Would it be possible in this scheme to mark strict mode per class? I could mark my own code as being strict and therefore get compile time failures when I make a typo myself, but wouldn't have to enforce that on all third party code.

Instead of per class I'd like to be able to set it per (module)path. We use r10k and split the role and profile modules to their own modulepath (typically called 'site'), all 3rd party modules live in the standard modulepath.

I could also imagine people wanting to set this per environment.

Lastly, I think --strict=ignore should be --strict=off, but that's personal preference.

great idea, hope to see it come to life sooner rather then later :)

cheers,

Walter

Ryan Whitehurst

unread,
Feb 23, 2016, 6:28:04 PM2/23/16
to puppe...@googlegroups.com
On Tue, Feb 23, 2016 at 3:22 PM, Walter Heck <walte...@olindata.com> wrote:
> On Tuesday, February 23, 2016 at 11:31:18 PM UTC+1, Ben Ford wrote:
>>
>> Would it be possible in this scheme to mark strict mode per class? I could
>> mark my own code as being strict and therefore get compile time failures
>> when I make a typo myself, but wouldn't have to enforce that on all third
>> party code.
>
>
> Instead of per class I'd like to be able to set it per (module)path. We use
> r10k and split the role and profile modules to their own modulepath
> (typically called 'site'), all 3rd party modules live in the standard
> modulepath.
>
> I could also imagine people wanting to set this per environment.
>
> Lastly, I think --strict=ignore should be --strict=off, but that's personal
> preference.

I agree with everything Walter is saying here. Per-class would be
difficult, but if there was a way to set it per-modulepath, you could
accomplish what Ben is asking for. It should also definitely be an
available setting in environment.conf.

--strict=off seems a bit nicer to me as well than does
--strict=ignore, but I don't care much.

My first thought is that --strict=warn would make a sensible default
in this scenario, as it's most similar to puppet's traditional
behavior of showing deprecation warnings every time.

> great idea, hope to see it come to life sooner rather then later :)
>
> cheers,
>
> Walter
>
> --
> 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+...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/puppet-dev/6c8709a0-30ae-43d8-b558-3819b4b5dd85%40googlegroups.com.

Rob Nelson

unread,
Feb 23, 2016, 8:24:02 PM2/23/16
to puppe...@googlegroups.com
Per module path would be good for site modules, but not great for forge modules. Maybe both could be supported.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CAHTHiAE8pFAA-O0Zf6d_MJrgiyxqw72s_YcXizHAe%3Di9-2vBzg%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.


--

Henrik Lindberg

unread,
Feb 23, 2016, 8:56:41 PM2/23/16
to puppe...@googlegroups.com
On 23/02/16 23:31, Ben Ford wrote:
> Would it be possible in this scheme to mark strict mode per class? I
> could mark my own code as being strict and therefore get compile time
> failures when I make a typo myself, but wouldn't have to enforce that on
> all third party code.
>

Good idea, you probably also need to be able to override because you may
want to assert the quality of things you are using from 3d parties,
either report the issues or fix and contribute back.

Henrik Lindberg

unread,
Feb 23, 2016, 8:58:53 PM2/23/16
to puppe...@googlegroups.com
On 24/02/16 00:22, Walter Heck wrote:
> On Tuesday, February 23, 2016 at 11:31:18 PM UTC+1, Ben Ford wrote:
>
> Would it be possible in this scheme to mark strict mode per class? I
> could mark my own code as being strict and therefore get compile
> time failures when I make a typo myself, but wouldn't have to
> enforce that on all third party code.
>
>
> Instead of per class I'd like to be able to set it per (module)path. We
> use r10k and split the role and profile modules to their own modulepath
> (typically called 'site'), all 3rd party modules live in the standard
> modulepath.
>
> I could also imagine people wanting to set this per environment.
>

Need to think about that for a bit. It makes it a lot more complicated.
Multiple paths etc. and where to specify them etc.

> Lastly, I think --strict=ignore should be --strict=off, but that's
> personal preference.
>

I think that is a good point. Will use 'off' instead of 'ignore'.

> great idea, hope to see it come to life sooner rather then later :)
>

There is a PR up already :-)

Henrik Lindberg

unread,
Feb 23, 2016, 9:04:01 PM2/23/16
to puppe...@googlegroups.com
On 24/02/16 00:27, Ryan Whitehurst wrote:
> On Tue, Feb 23, 2016 at 3:22 PM, Walter Heck <walte...@olindata.com> wrote:
>> On Tuesday, February 23, 2016 at 11:31:18 PM UTC+1, Ben Ford wrote:
>>>
>>> Would it be possible in this scheme to mark strict mode per class? I could
>>> mark my own code as being strict and therefore get compile time failures
>>> when I make a typo myself, but wouldn't have to enforce that on all third
>>> party code.
>>
>>
>> Instead of per class I'd like to be able to set it per (module)path. We use
>> r10k and split the role and profile modules to their own modulepath
>> (typically called 'site'), all 3rd party modules live in the standard
>> modulepath.
>>
>> I could also imagine people wanting to set this per environment.
>>
>> Lastly, I think --strict=ignore should be --strict=off, but that's personal
>> preference.
>
> I agree with everything Walter is saying here. Per-class would be
> difficult, but if there was a way to set it per-modulepath, you could
> accomplish what Ben is asking for. It should also definitely be an
> available setting in environment.conf.
>

By per modulepath, do you mean per directory containing modules?
the module path consists of multiple directories and the environment has
one.

Would adding strictness to the module metadata.json work for you?
Then each module author decides how clean their code is.

> --strict=off seems a bit nicer to me as well than does
> --strict=ignore, but I don't care much.
>
I also bought the --strict='off' since in many cases it also actually
turns of the checking (but not always, and it may just be a
"ignore"/suppression of the logging.

> My first thought is that --strict=warn would make a sensible default
> in this scenario, as it's most similar to puppet's traditional
> behavior of showing deprecation warnings every time.
>

In the PR that is up, --strict=warning is the default.

Trevor Vaughan

unread,
Feb 24, 2016, 5:49:17 AM2/24/16
to puppe...@googlegroups.com
I'm also a fan of per module which can override a global setting.

If it could be part of the metadata.json, that would be ideal and would allow for attestation on the Forge if appropriate.

The global --strict=off should override any module-level setting.

Thanks,

Trevor

--
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+...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--
Trevor Vaughan
Vice President, Onyx Point, Inc
(410) 541-6699

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

Henrik Lindberg

unread,
Feb 24, 2016, 8:58:24 AM2/24/16
to puppe...@googlegroups.com
On 24/02/16 11:49, Trevor Vaughan wrote:
> I'm also a fan of per module which can override a global setting.
>
> If it could be part of the metadata.json, that would be ideal and would
> allow for attestation on the Forge if appropriate.
>
> The global --strict=off should override any module-level setting.
>


I have been thinking about this for a bit now. Unfortunately it is going
to be very difficult to achieve any sort of "per module" handling of
strictness. While it can be applied statically to parsing, most of the
deprecations are about semantic problems that occur at evaluation time
and in places where there is absolutely no clue about the origin of the
logic - or for that matter a mix of modules and data sources that
together resulted in a state that in the future will be an error.

I think we will start with just a global --strict flag that can be used
in combination with tools like puppet parser validate at authoring time
to catch problems. In production I think you always set --strict=off
since you must already have reviewed and tested the logic that is in
production, being reminded about warnings for all nodes every 30 minutes
is not really what you want.

It makes a lot of sense to set it per environment. We do have issues
with that though; when you want to treat things differently in
development and production as you must modify a file that is checked in
and you must remember to not check in the settings that are only
applicable when an environment is in test or development state. Still
better than just a global flag.

- henrik

Ryan Whitehurst

unread,
Feb 24, 2016, 2:23:46 PM2/24/16
to puppe...@googlegroups.com
On Wed, Feb 24, 2016 at 5:58 AM, Henrik Lindberg
<henrik....@puppetlabs.com> wrote:
> On 24/02/16 11:49, Trevor Vaughan wrote:
>>
>> I'm also a fan of per module which can override a global setting.
>>
>> If it could be part of the metadata.json, that would be ideal and would
>> allow for attestation on the Forge if appropriate.
>>
>> The global --strict=off should override any module-level setting.
>>
>
>
> I have been thinking about this for a bit now. Unfortunately it is going to
> be very difficult to achieve any sort of "per module" handling of
> strictness. While it can be applied statically to parsing, most of the
> deprecations are about semantic problems that occur at evaluation time and
> in places where there is absolutely no clue about the origin of the logic -
> or for that matter a mix of modules and data sources that together resulted
> in a state that in the future will be an error.

I'm not even completely sure what the semantics of "per-module" would
be in practice anyway. On the forge, like Trevor suggested, any code
quality metric that uses this should always be checking everything
that --strict checks; you shouldn't be able to say "no, I want to
write less-than-ideal or outdated code and have my module appear to
have the same code quality as modules that follow latest best
practices". In actual deployments (whether testing or production),
saying "I only want this information from my custom code, not these
forge modules" is useful, but the semantics of a global setting and
per-module would be hard. How would I say both "I want deprecation
warnings for all my code unless the author disabled strict checking
because I'm practical and don't need large swaths of warnings for code
I didn't write that just add noise" and "I want to force deprecation
warnings everywhere because I'm upgrading major versions and if I
don't catch these now it will be a failure when I upgrade"?

I think global and per-environment are sufficient. Most of the time, I
suspect --strict=warn is going to be all you want anyway.

> I think we will start with just a global --strict flag that can be used in
> combination with tools like puppet parser validate at authoring time to
> catch problems. In production I think you always set --strict=off since you
> must already have reviewed and tested the logic that is in production, being
> reminded about warnings for all nodes every 30 minutes is not really what
> you want.

This makes sense. If you had all your code to the point where it
generated no --strict warnings/errors, you might use --strict=error to
enforce keeping it that way. As well, if you don't have (sufficient)
tests for your code (like a lot of internal code), you might use
--strict=warn so that those warnings were surfaced somewhere even if
you missed certain cases.

> It makes a lot of sense to set it per environment. We do have issues with
> that though; when you want to treat things differently in development and
> production as you must modify a file that is checked in and you must
> remember to not check in the settings that are only applicable when an
> environment is in test or development state. Still better than just a global
> flag.

This isn't very different from setting the parser via environment.conf
in 3.8, so I think it's fine. Lots of people still don't use some sort
of vcs branching workflow,, so this issue wouldn't really affect them
anyway.

Walter Heck

unread,
Feb 24, 2016, 2:24:05 PM2/24/16
to Puppet Developers
On Wednesday, February 24, 2016 at 11:49:17 AM UTC+1, Trevor Vaughan wrote:
I'm also a fan of per module which can override a global setting.

If it could be part of the metadata.json, that would be ideal and would allow for attestation on the Forge if appropriate.
I gave this some thought and I would be heavily against this: a module doesn't get to decide for it's consumers wether they will want to run in strict mode. The person running the puppetmaster should decide this, preferrably on a per environment base.

What I do think would be good to have in metadata.json is a flag that says which modes the module supports for which puppet versions. That way it can be displayed on the forge for instance and I can choose in advance to only download modules supporting strict mode.

cheers,

Trevor Vaughan

unread,
Feb 24, 2016, 4:12:48 PM2/24/16
to puppe...@googlegroups.com
+1 to the mode support in the metadata.json

Dependency checking will be more fun.

Trevor

--
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+...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

Henrik Lindberg

unread,
Feb 24, 2016, 11:35:42 PM2/24/16
to puppe...@googlegroups.com
Good idea. Ideally the forge should validate the module by parsing
everything in it and automatically assert if it passes strict without
warnings for the version the module states it supports.

This would be a reasonable measure of quality.

Forge can however not validate the runtime strictness (it would need to
evaluate / compile and that is not guaranteed to find all such problems.

We naturally want to make as much as possible be found when parsing and
validating statically.

If the forge ere to check strictness for statically discoverable things,
would you need to manually state strictness in metadata.json?

- henrik

> cheers,
>
> --
> 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+...@googlegroups.com
> <mailto:puppet-dev+...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/puppet-dev/2a0bec94-ff66-4e0f-9930-a76a193a5f88%40googlegroups.com
> <https://groups.google.com/d/msgid/puppet-dev/2a0bec94-ff66-4e0f-9930-a76a193a5f88%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.


--

Henrik Lindberg

unread,
Feb 24, 2016, 11:47:35 PM2/24/16
to puppe...@googlegroups.com
I am following up with a runtime type strictness thing.

If you have a construct like this in your manifests:

Notify['left'] -> $stuff -> Notify['right']

and at runtime $stuff happens to be an empty array, puppet currently
silently skips the middle part, and thus 'left' and 'right' are not
ordered via the dependency in the middle.

Should it warn? Is it an error? (I understand there will be a difference
of opinion here if it should always be one or the other, or if it should
be controlled by the --strict option). I just wanted to include it as an
example of something that is not caught by static checking at
parse/validation time.

Statically we could possibly check if the outcome is statically known
or if there is a potential problem (i.e. where evaluation can lead to
"empty thing in the middle"). But I am not sure what the quality of that
would be and how expensive it would be to implement.

There are number of such cases where puppet is trying to be helpful and
ends up with a messy situation that it silently ignores or goofs up on.

Gary Larizza

unread,
Feb 25, 2016, 12:50:15 AM2/25/16
to puppe...@googlegroups.com
Oh the chain syntax...   I have a couple of reactions:

If $stuff is an empty array, it's technically NOT undef, so it has a 'value', but I dunno why you'd put that in a dependency chain.  In Puppet, I like that dependencies are expressed for a reason - you went to the effort to express it because it was necessary.  So if you did the above and a dependency was not established, my knee-jerk reaction is to consider it at LEAST unexpected behavior which should be warned (if not an error).  I'd consider it an error though - you intended a dependency and one was not established.  
 

Statically we could possibly check if the outcome is statically known
or if there is a potential problem (i.e. where evaluation can lead to "empty thing in the middle"). But I am not sure what the quality of that would be and how expensive it would be to implement.

There are number of such cases where puppet is trying to be helpful and ends up with a messy situation that it silently ignores or goofs up on.


- henrik



--

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

--
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+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/56CE8764.2000703%40puppetlabs.com.

For more options, visit https://groups.google.com/d/optout.



--
Gary Larizza
Professional Services Engineer
Puppet Labs

Gary Larizza

unread,
Feb 25, 2016, 9:08:23 AM2/25/16
to puppe...@googlegroups.com
As I was thinking overnight, one hack we abused early on was this:

Package<|title == 'glibc'|> -> Class['fpm'] -> Class['apache']

The first being a search in the event the package was in the catalog, it should then have a dependency. If it is NOT, then no worries (this module shouldn't manage the package, but should come AFTER it if it IS in the catalog). This violates my response above and was something we needed largely before we had profiles (since that logic should go higher). I dunno whether this is "strict" or not.  

Henrik Lindberg

unread,
Feb 25, 2016, 9:43:03 AM2/25/16
to puppe...@googlegroups.com
On 25/02/16 15:08, Gary Larizza wrote:
>
>
> On Wednesday, February 24, 2016, Gary Larizza <ga...@puppetlabs.com
> <mailto:ga...@puppetlabs.com>> wrote:
>
>
>
> On Wed, Feb 24, 2016 at 10:47 PM, Henrik Lindberg
> <henrik....@puppetlabs.com
It is one of the troublesome cases for sure (there are more). In this
case the problem is you can do nothing to protect against an error since
you cannot have a condition on the query returning anything and you have
given your ability to have any say in the matter leaving it do puppet to
divine the intention.

If puppet takes the intention as there is supposed to be dependencies
between things on the left to things on the right, then it should always
short-circuit the empty man in the middle. Then neither of these cases
are strictness violations (except in the reverse sense that "if you
expect this to not create dependencies when there is an empty man in the
middle you just got the opposite" (which we then warn about).

Trevor Vaughan

unread,
Feb 25, 2016, 9:55:01 AM2/25/16
to puppe...@googlegroups.com
Hmm.....

I think, as long as it is documented, then whatever behavior is deterministic is fine.
'
I think that there is value in the following resolutions:

Notify['left'] -> [] -> Notify['right']
  * Noop since there is nothing in []

Notify['left'] -> [] -> Notify['right']
Notify['left'] -> Notify['right']
  * First == Noop
  * Second == Expected ordering

Notify['left'] -> Undef -> <Whatever>
  * Error since Undef is in the relationship

I would *love* to see an extension of the autorequire, etc... capabilities in the core relationship language.

Notify['left'] --> Notify['right'] (Idea: double symbol means auto*. '--', '~~', etc...)
  * Notify happens as usual. If one, or both resources do not exist in the catalog, this is not an error.

Yes, I know that this starts to expose some 'spooky action at a distance' features but, let's face it, that ship has sailed with logic flying in all over the place with Exported Resources, Orchestrator, Hiera Backends, ENCs, etc...

Trevor

--
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+...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--

Hunter Haugen

unread,
Feb 25, 2016, 3:44:55 PM2/25/16
to puppe...@googlegroups.com
I am following up with a runtime type strictness thing.

If you have a construct like this in your manifests:

Notify['left'] -> $stuff -> Notify['right']


Thansk for asking! For me, I would prefer the ordering to follow left -> right, and not error or warn.

If I have expressed Notify['left'] -> $stuff -> Notify['right'] then the ordering of left then right should be possible in all circumstances. Whether it is required or not when $stuff is empty is another matter, but it doesn't impact the user if all resources succeed.

Technologically, the references could be explicit references or collectors, and collectors can produce zero references, and this is actually DESIRED by me, as recently as yesterday. My use case was chaining lots of collections of optional defined types Like `A <||> -> B <||> -> C <||> D <||>` where B/C/D are optional, but A must always come before B/C/D and D must always come after A/B/C. In minimal cases, only A and D may be declared, so I ended up doing A <||> -> D <||> ; A <||> -> B <||> -> D <||> ; A <||> -> C <||> -> D <||> which is overly verbose.

It's worth noting that I ran into the above case because I expected empty collectors to maintain the chain, and was surprised when resources were evaluated out of order.

Producing a whit or something similar to contain the list of zero or more references would give the dag the structure it needs to maintain ordering even when an element in the chain is empty.

Hunter Haugen

unread,
Feb 25, 2016, 3:52:35 PM2/25/16
to puppe...@googlegroups.com
On Thu, Feb 25, 2016 at 12:44 PM, Hunter Haugen <hun...@puppetlabs.com> wrote:
I am following up with a runtime type strictness thing.

If you have a construct like this in your manifests:

Notify['left'] -> $stuff -> Notify['right']


Thansk for asking! For me, I would prefer the ordering to follow left -> right, and not error or warn.

If I have expressed Notify['left'] -> $stuff -> Notify['right'] then the ordering of left then right should be possible in all circumstances. Whether it is required or not when $stuff is empty is another matter, but it doesn't impact the user if all resources succeed.

Technologically, the references could be explicit references or collectors, and collectors can produce zero references, and this is actually DESIRED by me, as recently as yesterday. My use case was chaining lots of collections of optional defined types Like `A <||> -> B <||> -> C <||> D <||>` where B/C/D are optional, but A must always come before B/C/D and D must always come after A/B/C. In minimal cases, only A and D may be declared, so I ended up doing A <||> -> D <||> ; A <||> -> B <||> -> D <||> ; A <||> -> C <||> -> D <||> which is overly verbose.

This is abstract, but maybe an example could help. For reference, my current manifest is https://github.com/hunner/puppetlabs-tomcat/blob/e3d8a56876cb072b60468ded86490051059a56a1/manifests/instance/dependencies.pp though is still a work in progress.

And this behavior could be taken off-thread and put in a ticket if you'd rather discuss the outcome separately, since it's not directly about warning and erroring on --strict

John Bollinger

unread,
Feb 25, 2016, 4:08:55 PM2/25/16
to Puppet Developers


On Thursday, February 25, 2016 at 8:55:01 AM UTC-6, Trevor Vaughan wrote:
Hmm.....

I think, as long as it is documented, then whatever behavior is deterministic is fine.
'
I think that there is value in the following resolutions:

Notify['left'] -> [] -> Notify['right']
  * Noop since there is nothing in []

Notify['left'] -> [] -> Notify['right']
Notify['left'] -> Notify['right']
  * First == Noop
  * Second == Expected ordering



I flip-flopped a bit on this.  I started from the position that it would be inappropriate for

Notify['left'] -> $stuff -> Notify['right']

or

Notify['left'] -> Stuff<||> -> Notify['right']

ever to fail to cause Notify['left'] to be applied before Notify['right'], as indeed it now does fail to do in the event that the stuff in the middle represents zero resources.  My basis there was that it is counterintuitive for such expressions to not establish relative ordering of the two Notifys.

Ultimately, however, that basis is completely subjective.  Others might reasonably intuit that such an expression would have exactly the semantics it actually does have, which are roughly equivalent to those of

Notify['left'] -> $stuff
$stuff
-> Notify['right']

.  Upon reflection, this alternative is less magic.  It falls out naturally from understanding that chain operators associate from left to right, and that each binary chain expression evaluates to the value of its right-hand operand.  That some people might find that behavior surprising is a consideration, but not a primary one for me.

Moreover, this isn't really all that novel a problem.  Consider, for instance, this Ruby code:

a = 0
b
= 1
c
= 2
print "oops" if a < b < c  # NoMethodError

Note also that this C analog actually prints the result you might naively expect:

#include <stdio.h>
int main() {
 
int a = 0, b = 1, c = 2;
 
if (a < b < c) puts("ok");
 
return 0;
}

.  Its output is the same if you swap a and b, though, yet it differs if you instead swap a and c.  (Hint for non-C-ers: the Ruby result gives a good clue to what happens in the C example.)

There might be more grounds for argument if the chain operators were just now being designed from scratch, but I see no justification for changing their reasonable and established behavior, especially since it is conceivable that the current behavior is in fact being intentionally leveraged at some sites.  I favor the resolutions Trevor presented.

 
Notify['left'] -> Undef -> <Whatever>
  * Error since Undef is in the relationship


The Undef case is to some extent a separate question.  I'm inclined to agree again that it should be an error for an operand of a chain operator to be undefined, but if that's not how it works now then I am ambivalent about whether the behavior should be changed.


John

Henrik Lindberg

unread,
Feb 25, 2016, 5:32:02 PM2/25/16
to puppe...@googlegroups.com
> right, and that each binary chain expression /evaluates to/ the value of
> its right-hand operand. That some people might find that behavior
> surprising is a consideration, but not a primary one for me.
>

It depends on the operator if it is left or right associative and if it
is cumulative. And also what the result of '(nothing) op (something)'
results in.

if
(nothing) -> X
X -> (nothing)

both evaluate to X

as in

0 + 1
1 + 0

Then empty man in the middle would be a noop.

If you consider the arrows to abstractly order a set of resources in an
array which at the end of the chain defines the partial order of the
resources it would be the same as doing array concatenation.

i.e.
[A] + [] + [B] = [A, B]

So - it is all down to definition of the arrow operators; i.e.
taste/most practical/least surprise.

Trevor Vaughan

unread,
Feb 25, 2016, 5:50:05 PM2/25/16
to puppe...@googlegroups.com
I just want to make sure I'm reading this right.

For the first scenario, noop means don't order anything, just do things in whatever order they happen.

And the second scenario is: don't fail B if A fails (don't create a relationship), but do A before B

Is this correct?

If it is, I favor the second scenario as the default as it is more intuitive to the 'ordered compile' of Puppet 3+.

Thanks,

Trevor

--
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+...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

Henrik Lindberg

unread,
Feb 25, 2016, 6:12:18 PM2/25/16
to puppe...@googlegroups.com
On 25/02/16 23:50, Trevor Vaughan wrote:
> I just want to make sure I'm reading this right.
>
> For the first scenario, noop means don't order anything, just do things
> in whatever order they happen.
>
> And the second scenario is: don't fail B if A fails (don't create a
> relationship), but do A before B
>
> Is this correct?
>

I have problems understanding what you mean by noop and fail here.

The fundamental question is, does this expression define the order of A
and B?

A -> [ ] -> B

You can view it either as it defines the order [A, B], or that it does
not and their partial order is unaffected.

If you like the answer to be [A, B], does it bother you that the
following is not the same thing:

A -> [ ]
[ ] -> B

As that would not order them - because [ ] here can be seen as either
first/last or "in the partial order that is defined elsewhere".

Trying to make that define the order [A, B] is impossible since every
resource starts out with the partial order [[], R, []] (nothing before
R, nothing after R).

- henrik


> If it is, I favor the second scenario as the default as it is more
> intuitive to the 'ordered compile' of Puppet 3+.
>
> Thanks,
>
> Trevor
>
> On Thu, Feb 25, 2016 at 5:31 PM, Henrik Lindberg
> <henrik....@puppetlabs.com <mailto:henrik....@puppetlabs.com>>
> <mailto:puppet-dev%2Bunsu...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/puppet-dev/56CF80DD.7010200%40puppetlabs.com.
>
> For more options, visit https://groups.google.com/d/optout.
>
>
>
>
> --
> Trevor Vaughan
> Vice President, Onyx Point, Inc
> (410) 541-6699
>
> -- This account not approved for unencrypted proprietary information --
>
> --
> 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+...@googlegroups.com
> <mailto:puppet-dev+...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/puppet-dev/CANs%2BFoUmvhYvCOift0Hn8vAGPyO89TmnyLty2mcB4R47h1fB3g%40mail.gmail.com
> <https://groups.google.com/d/msgid/puppet-dev/CANs%2BFoUmvhYvCOift0Hn8vAGPyO89TmnyLty2mcB4R47h1fB3g%40mail.gmail.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.


--

Trevor Vaughan

unread,
Feb 25, 2016, 8:45:46 PM2/25/16
to puppe...@googlegroups.com
Sorry about that.

What I mean is this:

A -> [] -> B is not equivalent to A -> B since failures in A will not affect B.

However, it would be equivalent to [A,B] which I am reading as A before B but not in an actual resource relationship.

And, it makes sense that A -> [] and [] -> B would not be related since they have no obvious syntax relationship.

Thanks,

Trevor

To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/56CF8A4D.3070004%40puppetlabs.com.

For more options, visit https://groups.google.com/d/optout.

John Bollinger

unread,
Feb 26, 2016, 2:54:07 PM2/26/16
to Puppet Developers


On Thursday, February 25, 2016 at 7:45:46 PM UTC-6, Trevor Vaughan wrote:
Sorry about that.

What I mean is this:

A -> [] -> B is not equivalent to A -> B since failures in A will not affect B.

However, it would be equivalent to [A,B] which I am reading as A before B but not in an actual resource relationship.


Now you're confusing me.  Chain expressions are a means for declaring resource relationships.  To the extent that failures of resources earlier in a chain prevent resources later in the same chain from being applied, that is a function of the resource relationships expressed by the chain.  If the chain A -> [] -> B does express a direct or transitive relationship between A and B then failures in A prevent B from being applied.  If that chain does not express any relationship between A and B then it does not constrain their order of application.

It sounds, however, as if you are asking for an altogether new kind of ordering constraint, one which directs order of application but allows resources to be applied despite the failure of resources constrained to be applied earlier.  If that's so, then I'm afraid I don't see the point.  If a resource B can by synced successfully despite another resource A failing to sync, then how can it matter whether the attempt to sync A is made before the attempt to sync B?

If indeed that's what you're asking for, however, then rather than create a new kind of relationship, I think it would be better to create a way to declare that application of specific containers always succeeds or always fails, regardless of the outcome of applying whatever is declared within.  Then you could wrap resource A in a container (e.g. a define) that succeeds whether A succeeds or not, and establish an ordinary relationship between that container and B.  I'm inclined to think that there would be other uses for such a feature, and rather than any new DSL syntax, the DSL interface to it could be implemented as a function.


John

Hunter Haugen

unread,
Mar 16, 2016, 1:06:08 PM3/16/16
to puppe...@googlegroups.com
On Thursday, February 25, 2016 at 7:45:46 PM UTC-6, Trevor Vaughan wrote:
Sorry about that.

What I mean is this:

A -> [] -> B is not equivalent to A -> B since failures in A will not affect B.

However, it would be equivalent to [A,B] which I am reading as A before B but not in an actual resource relationship.


Just to be a little more clear, when declaring resources I never use a KNOWN empty-in-all-cases array in resource chains, so using "[]" is a leaky analogy. Instead one example is:

Thing[a] -> Thing <| name == $::something::i::want |> -> Thing[b]

Some people assume that the Thing[$::something::i::want] is declared and if not then that is an error state.

Some people assume that the Thing[$::something::i::want] may be declared, but if not then that's fine.

But for all cases Thing[a] -> Thing[b] at most is desired, or at least is possible. The only case in which disjointed dependencies are logically different is when failures in Thing[a] do not skip the evaluation of Thing[b] if and only if Thing[$::something::i::want] is undefined. Does that occur in the real world? Not afaik.

-----

The other example that comes to mind is:

Thing[a] -> $might_contain_references -> Thing[b]

This differs from above as variables are parse-order dependent and collectors are not (right?), and variables may be undefined or undef and collectors are... neither of those things (instead the graph is updated directly when collectors return resources, I believe).

A thought on semantic differencs from collectors: the value of a variable is directly obveservable at evaluation time, and dependency chains can be created based on this evaluation. Therefore I would posit that undef or undefined variables in dependency chains are always an error state, whereas empty collectors may or may not be.

John Bollinger

unread,
Mar 18, 2016, 9:46:57 AM3/18/16
to Puppet Developers


On Wednesday, March 16, 2016 at 12:06:08 PM UTC-5, Hunter Haugen wrote:

On Thursday, February 25, 2016 at 7:45:46 PM UTC-6, Trevor Vaughan wrote:
Sorry about that.

What I mean is this:

A -> [] -> B is not equivalent to A -> B since failures in A will not affect B.

However, it would be equivalent to [A,B] which I am reading as A before B but not in an actual resource relationship.


Just to be a little more clear, when declaring resources I never use a KNOWN empty-in-all-cases array in resource chains, so using "[]" is a leaky analogy.


I think everyone participating in the discussion understands that, but the variation in possible behaviors happens only when the expression in the middle of the chain happens to evaluate to zero resources, as "[]" trivially does.  Please do not suggest that Puppet might exhibit different behavior when that expression can be evaluated statically than when it can be evaluated only via a full catalog build.

 
Instead one example is:

Thing[a] -> Thing <| name == $::something::i::want |> -> Thing[b]

Some people assume that the Thing[$::something::i::want] is declared and if not then that is an error state.

Some people assume that the Thing[$::something::i::want] may be declared, but if not then that's fine.



I hadn't considered that anyone might assume that it would be an error for the collector to collect zero resources under those circumstances, but I guess that's plausible.  I thought we had taken as given that collecting zero resources was ok, though, and that we were talking about what should be the meaning of the overall chain when one of the middle expressions evaluates to zero resources.,

 
But for all cases Thing[a] -> Thing[b] at most is desired, or at least is possible. The only case in which disjointed dependencies are logically different is when failures in Thing[a] do not skip the evaluation of Thing[b] if and only if Thing[$::something::i::want] is undefined.


I'm not sure I follow you there.  Under all circumstances the three element chain (currently) has exactly the same semantics as two two-element chains with respect to resource relationships, which are what determine at a low level how the resources are ordered and which are applied in the event that some fail.  Are you saying that you want there to be a difference, as Trevor seemed to say?

 
Does that occur in the real world? Not afaik.


That was indeed my conclusion too, though.  If Thing[b] can be synced applied despite Thing[a] not having successfully been synced, then it should not matter to Thing[b] whether any attempt has been made to sync Thing[a] -- that is, their relative order of application does not matter.
 

-----

 
The other example that comes to mind is:

Thing[a] -> $might_contain_references -> Thing[b]

This differs from above as variables are parse-order dependent and collectors are not (right?), and variables may be undefined or undef and collectors are... neither of those things (instead the graph is updated directly when collectors return resources, I believe).

A thought on semantic differencs from collectors: the value of a variable is directly obveservable at evaluation time, and dependency chains can be created based on this evaluation. Therefore I would posit that undef or undefined variables in dependency chains are always an error state, whereas empty collectors may or may not be.


I'm not sure about the details of the current behavior here, but I think you're raising a separate issue: how an expression with undefined value is interpreted when it appears as an operand of a chain operator.  The value of  a collector expression that happens to collect zero resources is well defined (as an empty collection), so this question is irrelevant to collectors in chain expressions.  I would be fine with undefined chain operands being an error, however.


John

Reply all
Reply to author
Forward
0 new messages