[chef] Re: Re: Re: Re: Re: Re: LWRP to extend a Chef resource?

[Daniel DeLeo < >]

On Wednesday, October 30, 2013 at 10:09 AM, Ranjib Dey wrote:

i dont agree that hwrp development will necessarily involve mucking around with internal api (given we dont have a clear understanding of what is internal api anyway). LWRP, like recipes & attributes ,, are instance_evaled. this way of extending has some inherent limitations. You cant do so many things. Testing itself becomes pain (shoutout the seth vargo more adding custom matcher api for lwrps). On contrast, lwrp's does not really stop you using chef internal api, so you have that risk even there.

If you subclass, it’s easy to depend on internal stuff without seeing any warning or error of any kind or using anything like `send` that overrides method visibility and such. For example:

    class A

      private

      def internal

        puts "oh noes"

      end

    end

    

    class B < A

      def external

        internal

      end

    end

    

    B.new.external

    

That code runs without any warning or error.

 

If you consider the example that was mentioned above, you create the resource in the action method itself. What if your lwrp has multiple actions? will you repeat that code? or create those resources in a commone place (chef calls it load_current_resource). What is you want to support whyrun mode and you want a more meaningful converge_by description instead of all the sub-resources' converge_by description? You cant do these things with lwrp.

You sure can, if you create your resources by instantiating them directly rather than the DSL. Is it still a “lightweight” provider at that point? I dunno.

But the point of this is to use composition instead of inheritance whenever feasible. As we saw above, inheritance implies tight coupling and provides direct access to internal APIs without any warning that this could be a problem.

I can cite many more examples. Recently we migrated the sumologic cookbook's sumo_source resource (this was a definition earlier) to a standard chef resource. We could not do this with lwrp, because you cant control the resource name with lwrp. with hwrp we could do that, and now we have rewritten the sumo_source without changing any of the integrations or consumers. We were able to keep the usage exactly same.

This is definitely a limitation of the current DSL (IMO) removing too much boilerplate, and something that could be easily fixed with the changes I mentioned earlier in the thread.

 

Its sad that you think we should not reuse chef's internal public api even after 5 years of using and extending it. Any public method from Chef's main domain object is its public facing api, and i'll encourage others to use it. We now have elaborate testing suite that can catch version specific breaking. And I dont know anyone who uses arbitrary chef version (almost all of us uses a specific version, and update/upgrade are tested before hand), chef 11.x minor releases have already proved that legit things can break even within minor release (so we dont have to muck around with internal api to experience the same effect), making it impossible for many of us to update. On contrary i think Chef committers should ensure api compatibility, throw deprecation warning in between introduction of breaking changes (and I know in most cases they do that).    

We should chat about this at the community summit, with a focus on coming up with a community driven plan to improve clarity around what classes and methods are available for public use and which are not. Past discussions around this have basically concluded with “I want Opscode to document the ruby api,” but looking at all the things we want to improve in Chef, this isn’t gonna be the top priority any time soon. If we focus on incremental improvement and making it easy for outsiders to contribute to this effort, we can improve the situation dramatically.

i really find it bizarre that we see so many recipe with Chef::Recipe.send(:include, AwesomModule::Foo) which is outright breaking encapsulation, and then defend instance_eval style extension instead of  require/subclass etc.

again, i love lwrp , its short, sweet., easy to learn but there are ample reasons to build hwrp. and this is not a rant, but a disagreement that extensions should be limited to the dsl. 

I don’t see what instance_eval has to do with it. If you’re talking about how LWRPs evaluate code to define a class, it’s actually not that hard to invoke the magic yourself: https://github.com/opscode/chef/blob/master/lib/chef/provider/lwrp_base.rb#L83 (it used to be a real PITA in Chef 10 and before).

-- 

Daniel DeLeo