On Mon, Sep 9, 2013 at 10:08 AM, John Bollinger
<john.bollin...@stjude.org>wrote:
> On Thursday, September 5, 2013 4:31:07 PM UTC-5, Jeff McCune wrote:
>
>> On Tue, Sep 3, 2013 at 12:44 PM, Luke Kanies <lu...@puppetlabs.com>wrote:
>>
>>> +1 to a function expressing containment but not declaring the class
>>> (#3). It's the only option of the 4 provided that doesn't have the
>>> side-effect of adding the class to the catalog. I've run across situations
>>> where a module may want to contain a third party class and relies on some
>>> other module to declare the class. "site" modules are a common example.
>>>
>>>
>>> Sorry, why? What's the benefit of requiring a second call? Or, what's
>>> the downside of defaulting to 'include' for classes that have not yet been
>>> evaluated?
>>>
>>
>> The concern of adding a class to the catalog is distinctly separate from
>> the concern of containing a class. Smashing the two concerns together
>> makes it difficult to address all of the concerns.
>>
>
>
> Are containment and declaration of a given class really separate
> concerns? One class cannot contain another if the other is never declared,
> so I would argue that they are *not* separate. Inasmuch as containment
> is an order-of-application issue, and Puppet has had 'require' and 'before'
> resource metaparameters to manage order of application since the
> introduction of parameterized classes, I would furthermore argue that
> whether or not declaration and containment *should* be considered
> separate in some abstract sense, in practice they never have been separate.
>
OK, I'm willing to adopt the perspective that containment and declaration
are not separate concerns from the customer point of view since I'm
definitely not an authority on this perspective.
I do think they're separate concerns from a maintainer point of view
though, since tightly coupling them makes it more difficult to change the
behavior of include() if it's directly called by contains() without
exception, but this matters much less to me in comparison to the customer
point of view.
> The benefit of requiring a second call is that the concerns are much
>> easier to address directly without edge cases and qualifiers for class
>> parameters.
>>
>
>
> Do you see specific edge cases or qualifiers that do not already exist?
>
Nope, just the existing issue with parameterized classes needing to be
evaluated before any other form of inclusion.
I mentioned a site module. Consider this example. What is the result?
>>
>> 1 class site::motd($template = 'site/motd') {
>> 2 notify { 'motd': message => "template is $template" }
>> 3 }
>> 4
>> 5 class goal {
>> 6 notify { "site::motd::template":
>> 7 message => "::site::motd::template is ${::site::motd::template}
>> (should be ${::node_template})"
>> 8 }
>> 9 }
>> 10
>> 11 class container {
>> 12 contain('site::motd')
>> 13 }
>> 14
>> 15 node default {
>> 16 $node_template = 'site/motd.jeff'
>> 17 class { 'container': }
>> 18 -> class { 'goal':}
>> 19
>> 20 class { 'site::motd': template => $node_template }
>> 21 }
>>
>> The result I expect is that the class declaration on line 20 trumps all
>> other includes because it is the most specific regarding explicit class
>> parameters.
>>
>>
>
> I cannot express how much I would love it if parameterized-style class
> declarations worked like that. As far as I'm concerned, the fact that they
> don't work that way is the headliner of the example and the root of the
> problem.
>
Yep.
> Indeed, as long as we're talking separation of concerns, the fact that
> parameterized-style declarations don't currently work that way is based in
> a separation of concerns problem itself: parameterized-style declarations
> both declare a class AND bind values to its parameters. There are
> alternative, uncoupled mechanisms for each of those, so they are clearly
> separate.
>
Yes, through issues like this I've come to understand why the concept of a
binding is itself worthwhile to provide in the implementation.
It's not as clear that containment and inclusion are separate. You can
> write a contain() function that does not provide declaration semantics, but
> it must fail if the named class is not declared. Moreover, that would make
> 'contain' tricky to implement such that its use does not create an
> evaluation-order dependency -- exactly the same sort of problem that the
> example presents, in fact. It could probably be done, but it would require
> lazy evaluation, which would have to contend with all the other lazy
> evaluation that Puppet performs, some of which can affect whether the
> function must succeed or fail.
>
>
>
>> The behavior we actually get with Puppet right now surprises me because
>> I'm quite confident that we decided include was fine to use along with
>> resource style class declarations so long as there is only one resource
>> style declaration...
>>
>
>
> It has never been quite that way, and that has always been my greatest
> objection to parameterized-style class declarations. They can be used
> together with include/require, but only if the one allowed
> parameterized-style declaration is evaluated before any of the
> non-parameterized ones.
>
>
>
>> However, what we actually get is this:
>>
>> (3.3.0-rc2-16-gcfe704e)
>> $ puppet apply -v contain.pp
>> Error: Duplicate declaration: Class[Site::Motd] is already declared;
>> cannot redeclare at /workspace/serious/src/puppet/**contain.pp:20 on
>> node agent1.example.org
>> Error: Duplicate declaration: Class[Site::Motd] is already declared;
>> cannot redeclare at /workspace/serious/src/puppet/**contain.pp:20 on
>> node agent1.example.org
>>
>> This is a bug in my opinion. The catalog should compile and apply and we
>> should see "::site::motd::template is site/motd.jeff (should be
>> site/motd.jeff)" printed out.
>>
>>
>
> That would be simply lovely. I'd buy the T-shirt. I'll even go file the
> bug ticket (using 'include' instead of 'contain') if you're saying it would
> be accepted.
>
> If that bug were fixed, would it assuage your concerns about a contain()
> function having class declaration semantics?
>
>
>
>> In any event, it's important that we allow one class to contain another
>> class and defer explicit class parameters to other places. If contains has
>> the side effect of declaring the class then this isn't possible unless the
>> end user carefully manages the parse order of their manifests.
>>
>
>
> This is really nothing new. On one hand, I feel confident in estimating
> the number of anchor-pattern analogues to your example in use outside test
> suites at roughly -1. On the other hand, users managing the parse order of
> their manifests has been par for the course since the introduction of
> parameterized classes, wherever parameterized-style class declarations are
> used. The problem is there, not inherently in the proposed semantics of
> contain().
>
Got it. This "contain but defer evaluation" concern is marginal.
> A separate, focused, function also keeps our support matrix small and
>>> more maintainable. We'll want to implement the behavior consistently
>>> across all the ways of adding classes to catalogs if we implement the
>>> behavior for one of them. These behaviors are burdensome to maintain over
>>> time. A contains() function allows the behavior to work with all the ways
>>> of adding classes to the catalog while allowing us to maintain the code in
>>> one supported manner.
>>>
>>>
>>> Can you elaborate? I don't know what you mean. What's burdensome about
>>> having 'contain' call 'include'?
>>>
>>
>> If contain calls include, how can you be sure some other module doesn't
>> declare the class before you when you want to provide explicit class
>> parameters yourself? You can't be sure unless you take on the burden of
>> carefully managing the parse order of the manifests to make absolutely sure
>> your resource style declaration happens before include, contains, require,
>> or whatever new way we invent to add classes to the catalog in an exclusive
>> way.
>>
>
>
> I think you're flipping cause and effect. Any mechanism that adds classes
> to the catalog in an exclusive way is inherently a source of the kind of
> parse-order problem you are (rightly) concerned about. That giving
> contain() declaration semantics would cause problems in conjunction with
> such ill-fitting mechanisms is a symptom of that deeper problem, not a
> problem in its own right. The same kind of problem would be felt by users
> of 'include', and even by users of resource-style class declaration syntax.
>
> From a user perspective, if contain() is implemented without declaration
> semantics then my own usage and my recommendation to everyone will be to
> ALWAYS precede contain() with include(). I can live with that, but my
> preference would be to avoid the need for it. From a software design
> perspective, I don't see a contain() function having class declaration
> semantics as commingling of separate of concerns.
>
Thank you for taking the time to reply so thoughtfully. Having your
perspective and insight is invaluable.
-Jeff
--
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+unsubscr...@googlegroups.com.
To post to this group, send email to puppet-dev@googlegroups.com.
Visit this group at http://groups.google.com/group/puppet-dev.
For more options, visit https://groups.google.com/groups/opt_out.
