[aologies, this got lost in my Drafts folder.]

On 28 February 2017 at 19:35, Erik Dalén <erik.gustav.da...@gmail.com>
wrote:

> Overall I think this looks pretty good, but I have some questions and
> comments.
>
> For implementations that can't enumerate all existing resources (at least
> not in any fast way), like for example File, should they fine a get method
> that returns an empty hash, nil or just not define a get method? nil or not
> defining would allow special handling of them, so that would probably be
> preferable.
>
> But shouldn't it really have a way to fetch the state of a single resource
> in for those implementations? How would this otherwise handle something
> like "puppet resource file /path/to/file", traverse the entire file system
> and stat every file?
>

Yup it should. I've added a short discussion of the problem to the "Known
limitations" section. I could add verbiage around the get() method
returning nil (as opposed to []) to signal that there might be instances
available, but it's not possible to enumerate them.


> It seems it would also be handy with some helper method that compared
> current_state & target_state to filter out the changes that actually need
> to happen. I would imagine that sort of code would otherwise be duplicated
> in many implementations.
>

True. Porting the existing types and providers from our supported modules
is high on my list to validate the design, and will expose the patterns.
That'll make it much easier to build a library of helpers.


> logger.attribute_changed could really have a default value for message if
> the implementation provides attribute, old_value & new_value they shouldn't
> all need to supply "Changed #{attribute} from #{old_value} to #{new_value}
> ".
>

That's already specified.


> Will the attribute type fields be able to use type definitions shipped in
> the same module or present on in environment?
>

Currently anywhere "puppet 4 data types" are mentioned, only the built-in
types are usable. This is because the type information is required on the
agent, but puppet doesn't make it available yet. This work is tracked in
[PUP-7197](https://tickets.puppetlabs.com/browse/PUP-7197), but even once
that is implemented, modules will have to wait until the functionality is
widely available, before being able to rely on that.


> Will they be able to inspect the catalog or communicate between
> implementations, like in the concat & concat_fragment case?
>

 My preferred method to solve that would be a new top-level plugin, that
can inspect and change the catalog after it has been built. That plugin
could then collect all the fragments, and build a file resource from them,
before the catalog is even passed to the agent.


> Will they be able to generate resources? That could probably be done with
> something similar to the logger methods, but there is nothing like that in
> the proposal. I assume they won't have a generate method.
>

Adding more discrete functionality and life-cycle methods later seems like
an easy extension, that will not require a major API revision. Also
possibly another use-case for a catalog transform plugin.


> Will there be some requirement that these implementations are thread safe
> so puppet can start managing multiple resources in parallel? Or will they
> run in some subprocess even?
>
>
Theoretically, those implementations could be run in a separate process.
Looking at things like Lutterkort's libral, this might happen quicker than
I think today. As a firm believer of small incremental steps, I only want
to speculate on those things as far as I need to avoid blocking a future
change. Therefore I'm focusing this new API mostly on being small, self
contained, and decoupled.


Thanks for your time and work in reviewing this!

Regards, David

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to puppet-users+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/puppet-users/CALF7fHZBTSFdjrv12wfL3Qh6Fb1pzX0eZ-cL8ze-_YwcChf3jg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to