[[chef-dev]] Re: [[chef-dev]] Re: [[chef-dev]] Resources, Providers and Runners, oh my - A tale of concerns

[Daniel DeLeo < >]

Ohai Chefs!

Mathias,

To follow up on our conversation on IRC today, what you're seeing in the LWRP spec failures results from the resources being initialized differently in the specs than they would be if created via the DSL. In the DSL, you see resources initialized this way:

args << @collection

args << @node

resource = Chef::Resource.const_get(resource_camel_case_name).new(*args)

But in the failing specs, you see this:

Chef::Resource::LwrpFoo.new("morpheus")

When you moved Runner#build_provider to Resource#build_provider, it became necessary for the specs to pass the resource collection to the resources when initializing. So the issue can be fixed by changing the specs to more closely match the behavior that would occur in a real chef run, for example:

rc = Chef::ResourceCollection.new

injector = Chef::Resource::LwrpFoo.new("morpheus", rc)

By the way, should you encounter errors due to @definitions being undefined somewhere, there are fixes for that in the works, so I'd recommend waiting for those fixes to hit master so we're not stepping on each others' toes.

As far as cleanly separating responsibilities, I'm all for it, and I'd say the amount of effort required to understand the code as it is now is good evidence that the effort is worthwhile.

HTH,

Dan DeLeo/kallistec

On Mon, Oct 12, 2009 at 4:38 PM, Christopher Brown < "> > wrote:

Adam originally wrote that code, and I'd like to wait for him to chime
in.  He'll be out of town for a couple more days, so I wanted to make
sure someone responded now to let you know someone will follow up with
real substance in a day or two.

I agree that a clean separation of concerns makes sense, but I'm
loathe to assert too quickly without Adam giving us his reasons behind
the current form.

Cheers,
Chris

On Mon, Oct 12, 2009 at 11:50 AM, Mathias Meyer < "> > wrote:
> Hi all,
>
> I'm trying to wrap my head around something, and I think I'm a bit
> stuck. While working on CHEF-606 [1] (which was a direct result of
> trying to fix CHEF-605 [2]) I ran across something I can only describe
> as mixed concerns.
>
> Chef::Runner has a similar functionality as Chef::Resource, they both
> have a run_action method, but other than I expected, Runner#run_action
> doesn't invoke Resource#run_action, it pretty much duplicates its
> functionality. The difference seems to be that when the Runner builds
> a provider instance it hands in some of its own instance variables
> (@collection, @definitions, @cookbook_loader) which the Resource knows
> nothing about, but Providers do. The Resource on the other hand only
> builds Providers without references to these, and its run_action
> method seems only to be used when running Resources directly from
> Recipes.
>
> That's where the concerns start to fall apart for me. I'd argue that
> the Runner shouldn't have to know anything about Providers, especially
> when all the logic dealing with them is already implemented in the
> Resource. But there's cases where the mentioned instance variables are
> expected to be set. That code seems to be mostly dealing with
> lightweight resources, as they are the only tests that failed. The
> code in RecipeDefinitionDSLCore references them, as far as I could
> tell it's used to create ad-hoc Recipes or something like that,
> correct me if I'm wrong.
>
> If I consider this a chain, where the Runner only knows about
> Resources, and the Resources are the only spot where the code deals
> with invoking Providers, there's a blind spot in the middle. Providers
> need access to objects the Resource doesn't know anything about. The
> question is if the Resource should know them, or if it should just
> hand them through. We started to move some code around, and play with
> some ideas, as can be seen in [3], but we're not happy with it. It
> basically hands the required objects into the Resource where they're
> just handed over to the Provider. The Resource though comes with its
> own instance variable @collection which doesn't seem to have any
> relation to the one the Provider refers to.
>
> I'm not exactly sure how to proceed on this, so I'm asking for some
> input, because maybe we missed something along the way, or maybe
> someone has other ideas how that can be fixed.
>
> For me, putting all the code that deals with running an action and
> notifying subscribed actions belongs into the Resource, it just seems
> to be the right place for it. But the fact that there's a mix of
> concerns here makes me think there's a bigger issue here.
>
> Let me know, what you think.
>
> Thanks!
>
> Cheers, Mathias
> [1] https://tickets.opscode.com/browse/CHEF-606
> [2] https://tickets.opscode.com/browse/CHEF-605
> [3] http://github.com/peritor/chef/tree/CHEF-606-WIP
> --
> http://paperplanes.de | http://holgarific.net
> http://twitter.com/roidrage
>

--
Christopher Brown, VP of Engineering
Opscode, Inc.
T: (425) 281-0727, E: ">
Twitter, IRC, Github: skeptomai