On 1 March 2017 at 13:46, Trevor Vaughan <[email protected]> wrote:
> As always, if I wait long enough, John will more eloquently provide the > response that I wanted to! > > I've added a few responses inline > > > *Big picture items* >> >> *Multiple implementations / implementation selection* >> >> In splitting resources into "definition" and "implementation", the >> proposal adheres to a *form* similar to the current system's, but it >> seems to have fundamentally different design goals. I've always >> interpreted the present type / provider system's separation of resource >> interface from implementation first and foremost as a means to accommodate >> multiple implementations. The one most appropriate to the target system is >> chosen from among those available. I think that's a sound design approach; >> I like it, and it has served Puppet well. As far as I can determine, >> however, the proposal loses that completely -- I see no means to support >> multiple implementations at all, much less means to match an implementation >> to the target system. >> >> > I hadn't actually caught this, but if not just an oversight, it is indeed > concerning. The ability to choose from various providers has been a strong > play in Puppet's favor. > See my answer to John's point. > > >> *Inadequate interface documentation* >> >> As I said earlier, one of the biggest problems with the current system is >> inadequate documentation. As it now stands, the proposal's documentation >> does about as well as the docs of the current system. Missing is >> information about how the runtime environment is intended to use the >> defined objects and methods -- for example, under what circumstances it >> will instantiate / invoke them, how many times per transaction (partially >> addressed in the doc), at what point(s) in the transaction. What may the >> environment do with the object returned by get() and its contents (i.e., >> must implementations assume that the environment may modify them)? Is >> there anything the environment is required to do with them? What may an >> implementation do with the object passed to set() and its contents? Is >> there any expectation about relationships between the objects provided by >> get() and received by put()? >> >> > This is definitely true. Gary's blog has been the seminal resource for > figuring out how to write types and providers and, instead of a new > interface, it might be good to finish the original docs. That said, I think > I remember David saying that part of the intent of this is to provide a > language-agnostic interface to managing the system so that users aren't > tightly bound to a particular implementation. I think that this is a good > goal *but* I would maintain compatibility with existing Type and Provider > implementations. > Existing types&providers will not go away for a long time. My prototype translates attributes into newproperty calls and shoves the get&set methods into a 3.x type. For everyone else in the agent or server this will look and behave exactly like a 3.x type. > > I've also put in requests for the ability to maintain global state across > the catalog compilation. If the brunt of the work is moved to the client, > this may be more feasible but I think that it's perfectly possible in the > current system with a namespaced caching system. > I've discussed this, and possible solutions in my answer to John. > > Also, I would like to see the resource utilization on the clients made > better and maybe this can be addressed during the update. > This interface provides the possibility for batching. The current agent runtime obviously cannot take advantage of that yet, but it is a chicken and egg issue. Getting this going would be a huge payoff. *Pushing new responsibilities onto resource implementations* >> >> The minimalistic approach to defining the interface for resource >> implementations has the inevitable effect of making the host environment >> incapable of some of the tasks that it presently performs, leaving the onus >> on resource implementations to fill the gaps. For example, since there is >> no way for the environment to retrieve the state of a specified individual >> resource, and because some resource types cannot feasibly be enumerated, >> the environment cannot, in general, determine whether resources are in >> sync. Only resource implementations can be relied upon to do that, and >> then only in the context of their put() methods. >> > > I'm assuming (possibly incorrectly) that there would be a standard Ruby > back end example provided. I'll also throw down the gauntlet and say that > it should be the File resource. Being one of the oldest, and most janky, > native types, I feel that it should be the seminal example. If you do > 'easy' ones, I'll look at this askance, if you tackle (and improve) the > core types, I'll have something solid to reference. > The v1 is not intended to solve all the nastiness that is required for the implementation of file. There are many use-cases that will never need that level of functionality. I'm also convinced that the current level of *complexity* in types and providers is not required for that level of functionality. > > >> >> Similarly, although I definitely like that a logging utility is made >> available to resource implementations, it seems that most responsibility >> for logging now falls on resource implementations as well. Currently, the >> environment handles a fair amount of that. It seems likely that moving the >> responsibility to resource implementations will not only make them more >> complicated to write, but will also reduce the consistency of log messages. >> > > Seems like a reasonable request for a stable exposed logging API > ? > > >> >> *Terminology Changes* >> >> The proposal seems to gratuitously change established terminology. What >> we now call "properties" are to be referred to as "attributes", which >> itself is currently used in a broader sense. "Types" become "definitions"; >> "providers" become "implementations". Why? However well or poorly you >> like the established terminology, it *is* established. The proposal >> seems to be offering variations on these existing things, not entirely new >> things, and changing all the terminology is likely to be more harmful than >> helpful. >> > > I've got to say that I didn't notice this until you mentioned it but, now > that you have, I'm also scratching my head. The current names make sense > and are documented. I do however, think that the new names are more > accessible to new users. > Thanks. > > >> >> *Specific features* >> >> *Attribute and parameter validation* >> >> Validation ought to be performed during catalog building, thus it needs >> to be adequately addressed by type "definitions". I am dissatisfied with >> the proposal's provision for that. The Puppet type system is very >> expressive, but it can also be very arcane. I maintain that it is a >> mistake to rely exclusively on the type system for validating "attributes" >> or "operational parameters". Moreover, I think the proposal is missing an >> opportunity to provide for multiple-attribute, multiple-parameter >> covalidation. This has been requested on and off for a long time, and this >> proposal is a great opportunity to finally put it in place. >> > > This was one of my biggest worries early on and it has been solidified > with the example code. The Type system simply isn't suitable for complex > validation at this point. I need logic (sometimes stupid amounts of logic) > and, until I can do arbitrary validation with the Type system, I can't use > it for my parameters. > Does that validation have to be server side? > Also, this is starting to look suspiciously like the MCollective DDL, > which doesn't bother me but I find amusing. > There is only so much you can do to a type system :-D > *Transaction progress and state* >> >> Resource implementations are afforded only indirect information about >> transaction context. For example, the docs say that "The implementation >> should not try to cache state beyond the transaction", but how, exactly, is >> the implementation supposed to recognize the end of a transaction? It >> would be highly desirable for implementations to have means to at least opt >> in to messages about transaction lifecycle and events. For some purposes, >> it would be useful to have a way for type implementations to be able to >> communicate with each other within the scope of a transaction. >> > > This is absolutely necessary! As much as we would like to keep things in > nice little boxes, they have interactions and we *need* those interactions. > Resources should know the following: > > 1) Where they are in the catalog compile stage and what they can access > 2) Where they are in the list of similar resource types and what they can > access > 3) Where they are in the graph and what they can access > > As we've grafted this ability into Puppet, we've gained the ability to do > thing like native 'concat' as well as rolling up resources to be able to > fail unless all resource data is properly defined. I very strongly believe > that a floating data type that is catalog accessible would be a good > fundamental addition to the Puppet language. > See my response to John. > > >> >> *Details* >> >> *"Read-only" vs. "init-only"* >> >> I understand -- I think -- the distinction you're drawing here, but it's >> confusing. Surely if a resource does not yet exist, then *all* its >> attributes are logically written when it is created / initialized. I could >> understand that is a plausible exception to read-onlyness if not for the >> explicit provision for init-only attributes. If you want to maintain both >> of these cases among the attribute metadata then please rename one of >> them. For example, perhaps "derived" or "internal" would describe what you >> mean by "read-only". >> >> *Compound resource names* >> >> I suppose that compound namevars are a topic that you hoped to avoid in >> your push for simplification, though it looks like they might not be hard >> to support via the interface you have already described. This is a weak >> area for the present system, and one that I think any replacement should >> support well and and fully. >> > > Fundamentally, Puppet uses relational database concepts in the basic > resource model and, if thought about as such, can use the usual tricks to > support uniqueness using multi-key resources. > See my response to John. > >> >> *Attribute / parameter clashes* >> >> By specifying attributes and operational parameters via separate hashes, >> the proposed system affords the opportunity for name collisions between >> these. That also makes resource definitions less cohesive with Puppet >> overall, because in the Puppet language, attributes and operational >> parameters are specified together, in the same syntactic construct. I >> fully understand the differences between these, but I suggest they not be >> reflected by a complete separation between their specifications, such as >> the proposal currently provides. >> > > I think I'm OK with this provided that the validator flags any name > collisions as a compile failure similar to what properties and parameters > do now. > > > >> >> *Resource implementations that become usable during a run* >> >> It is only relatively recent that Puppet implemented the long-desired >> feature of supporting resource providers that become usable as a result of >> other resources being managed earlier in the same run. It is not clear >> whether or how that would be supported under the proposal as currently >> presented, but this is a feature that I do not think we want to lose. >> > > Ditto. I really need this capability to remain. > yup. Thanks for your time and work spent reviewing this. Cheers, David -- 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 view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CALF7fHb_EdtAgLFJHenXxKJ6o-F_cpHrCDLL_5v6giGjFfCjPA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
