On Aug 30, 2013, at 4:33 PM, Patrick Carlisle <[email protected]> wrote:
> > On Fri, Aug 30, 2013 at 3:58 PM, Luke Kanies <[email protected]> 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 [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/puppet-dev. For more options, visit https://groups.google.com/groups/opt_out.
