poise / poise

A set of libraries for writing reusable Chef cookbooks
Apache License 2.0
106 stars 31 forks source link

lazy option_collector? #29

Open alicebob opened 7 years ago

alicebob commented 7 years ago

Hi,

I'm having some trouble with a lazy option_collector attribute value, and looking at the Poise code it seems there is no support for that. Am I right?

I want to use the :parameters option from the consul cookbook. Source of the resource: https://github.com/johnbellone/consul-cookbook/blob/dec40396bea00b3f24b966291bde399e1162d981/libraries/consul_definition.rb#L36

This is what I'm trying to do:

   consul_definition 'postgres' do
      type 'service'
      parameters(lazy{
        { tags: node[:mybook][:postgres][:tags].keys() }
      })
      notifies :reload, 'consul_service[consul]', :delayed
    end

This is the error I get:

         NameError
         ---------
         uninitialized constant Poise::Helpers::OptionCollector::ClassMethods::Exceptions

         Cookbook Trace:
         ---------------
           /tmp/kitchen/cache/cookbooks/poise/files/halite_gem/poise/helpers/option_collector.rb:119:in `block in option_collector_attribute'
           /tmp/kitchen/cache/cookbooks/mybook/recipes/_postgres.rb:82:in `block in from_file'
           /tmp/kitchen/cache/cookbooks/mybook/recipes/_postgres.rb:80:in `from_file'
           /tmp/kitchen/cache/cookbooks/mybook/recipes/eventdb.rb:66:in `from_file'
           /tmp/kitchen/cache/cookbooks/mybook/recipes/everything.rb:33:in `from_file'

         Relevant File Content:
         ----------------------
         /tmp/kitchen/cache/cookbooks/poise/files/halite_gem/poise/helpers/option_collector.rb:

         112:                  arg = case parser
         113:                        when Proc
         114:                          instance_exec(arg, &parser)
         115:                        when Symbol
         116:                          send(parser, arg)
         117:                        end
         118:                end
         119>>               raise Exceptions::ValidationFailed, "Option #{name} must be a Hash" if !arg.is_a?(Hash)
         120:                # Should this and the update below be a deep merge?
         121:                value.update(arg)
         122:              end
         123:              if block
         124:                ctx = OptionEvalContext.new(self, forced_keys)
         125:                ctx.instance_exec(&block)
         126:                value.update(ctx._options)
         127:              end

The documentation for lazy suggests that what I'm trying to do should work for non-poise use: https://docs.chef.io/resource_common.html#lazy-evaluation

Any ideas? Thanks!

coderanger commented 7 years ago

Yeah, the optioncollector helper is from before the lazy{} helper worked on instances. There doesn't look to be a great solution here other than fixing it in Poise itself.

alicebob commented 7 years ago

Thanks @coderanger!

Would it be a matter of adding

arg = instance_eval(&arg) if arg.is_a?(Chef::DelayedEvaluator)

right after this if: https://github.com/poise/poise/blob/master/lib/poise/helpers/option_collector.rb#L110 ?

I can try that, but I have no idea where the tests for this have to go.

coderanger commented 7 years ago

Unfortunately no, that would fix the error but you wouldn't actually get the delayed evaluation behavior (i.e. the eval would happen at set time). It needs a bigger rewrite :-/ The way the lazy{} helper in that use case works is by delaying the eval until acess, rather than running it on set. Basically the whole helper as written needs to be scrapped and rewritten using the newer Property API which was built explicitly to make this easier for me in the future but that requires time on my part as well as either backporting the Property system or dropping support for early 12.x releases.

alicebob commented 7 years ago

Ugh. Makes sense that doesn't work. I'll see if I can solve this some other way; I'm sure I can somehow reorder things as a workaround.

Thanks for the quick help!

(I'll leave the issue open in case someone else runs into the same problem. Feel free to close as you see fit)

alicebob commented 7 years ago

btw, there seems to be something wrong with the exception above, that might be fixable :)

coderanger commented 7 years ago

Check if poise-derived might help fix the root issues you are having. Worth a shot at least.