On Aug 30, 2013, at 4:33 PM, Patrick Carlisle <patr...@puppetlabs.com> wrote:
>
> On Fri, Aug 30, 2013 at 3:58 PM, Luke Kanies <l...@puppetlabs.com> wrote
> Hmm. I'm essentially positive that there was a version of this bug that was
> caused by whits cancelling out when classes were empty, but it sounds like
> that form has been fixed. Either that, or the first time I did a deep dive
> on this, I came away with a completely broken idea of the problem, which is
> absolutely a possibility, especially given how long this has been sitting
> around unsolved.
>
>
> It's bug #3382, but it's marked resolved and seems to still work.
Ah, I missed this fact; it somehow transitioned into "the anchor pattern
problem" for me.
>>> Also I think fixing this only for the new resource like syntax and not for
>>> include would be a mistake - though i can see why that would be the chosen
>>> path as doing it otherwise would easily create loops etc.
>>>
>>>
>>> The reason for using the resource syntax to enforce containment is because
>>> something can only be contained in one place. If containment is allowed in
>>> multiple places you end up with inexplicable ordering and loops and are
>>> back at #2423. Include has come to mean "I want this in the catalog with no
>>> dependencies", and so adding containment to that would be a problem. There
>>> is also the possibility of some new syntax to allow a "contained" class (a
>>> function could be done, but then parameters would need to be passed in a
>>> hash and it would be a bit odd).
>>>
>>> Here are the possibilities:
>>>
>>> * resource like syntax for classes expresses containment:
>>>
>>> class container { class { contained: parameter => value } }
>>>
>>> * a function declares the class *and* expresses containment
>>>
>>> class container { contain(contained, { parameter => value }) }
>>>
>>> * a function expresses containment, but does not declare the class
>>>
>>> class container { class { contained: parameter => value }
>>> contain(contained) }
>>>
>>> * a new "resource type" expresses containment
>>>
>>> class container { contain { contained: parameter => value } }
>>>
>>> The two functions could actually be written right now and distributed in a
>>> module, they just need to make sure that when the class is added to the
>>> catalog that it also adds an edge from the container to the contained class
>>> (Patrick spiked this and verified that it works).
>>>
>>> Now, the reason that I think that the existing resource syntax is the right
>>> way is that it already allows the class to appear in one place, which is
>>> what we need for reasonable containment. This also gives classes
>>> containment of the exact same form as resources (because they currently get
>>> contained in their class), which is a good symmetry to have. In addition,
>>> there has been talk about allowing duplicate resources in the catalog,
>>> which will open up the whole containment question for resources (what does
>>> it mean for the resource to be in two places?). To solve that we will
>>> probably need some way of "declaring without containment" for resources,
>>> and having classes and resources agree on how to "declare *with*
>>> containment" will make any solution there transfer better to both classes
>>> and resources.
>>
>> I still don't understand why you wouldn't just use dependencies instead of
>> containment here. It seems like either edge type works fine, so why focus
>> on containment?
>>
>>
>> Containment is just another form of dependencies, but with slightly
>> different meaning. If class A contains something then it is more than a
>> simple requires relationship because it means that anything that requires A
>> (Class[a] -> Class[b]) needs to wait until everything that A contains has
>> been executed (that is the transitive requires). That direction works with a
>> simple requires.
>>
>> The other direction doesn't work with a simple requires. If class B,
>> instead, needs to happen before class A (Class[b] -> Class[a]) the normal
>> mental model is that nothing that A contains will happen until B is
>> complete, then you would need to setup that relationship with everything
>> contained inside A explicitly, which breaks encapsulation.
>>
>> The anchor pattern does this by setting up a resource in A, which will be
>> contained using the above semantics, to explicitly setup relationships with
>> classes that A wants to contain. These relationships are exactly what the
>> relationship graph does when it is given a containment relationship.
>>
>> The semantics are:
>>
>> * Requires: A requires B means that all of A will be executed before B is
>> executed
>> * Contains: A contains B means that any resource C that requires A will
>> only be executed after B is executed as well as any resource D that is
>> required by A will be executed before B is executed
>
> Do you think John's recommendation of a new 'contain' function would solve
> this? If not, why not?
>
> A function that adds containment edges should work.
>
> We can't change the default semantics of 'include' or class declaration to
> result in containment; the periodic cycles will just come right back, won't
> they?
>
> We can change resource style class declaration, but not include (and changing
> one without the other feels like a hack to me). If you have multiple includes
> then the class would only be contained once, and where it's contained would
> be dependent on the order of execution of the include function. This results
> in unexpected behavior really quickly (e.g. a class being completely applied
> before the class it supposedly contained). If you make include add a
> dependency in every location then I think you'd have the cycles come back.
>
>
> If the new function doesn't work, what about a new top-level container that
> defaults to containing, rather than the current model? Or what about both?
>
> I'm definitely entertaining both right now. I'm not sure I've thought through
> the implications of a new top-level container enough yet, but it's appealing.
> A function would be easier of course.
Frankly, I would prefer both. We could ship the 'contain' function basically
immediately, because it's like a 2 line function (call include, add the edge).
The new top-level container then is basically just syntactic sugar (includes in
them are equal to contains elsewhere; or maybe we disallow include in them?).
I'm hoping to submit an ARM soon to cover the overall justification for them.
--
Luke Kanies | http://about.me/lak | http://puppetlabs.com/ | +1-615-594-8199
--
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.
