diff options
| author | danielsdeleo <dan@getchef.com> | 2014-07-29 17:14:24 -0700 |
|---|---|---|
| committer | danielsdeleo <dan@getchef.com> | 2014-07-30 14:07:07 -0700 |
| commit | 263f62774641bf32be6fd79d1d69022531fb3285 (patch) | |
| tree | 4e7a541754dd2e81e64c8ca28f77360368062188 /lib/chef/policy_builder | |
| parent | fc3b0eed9b8b33bcb51121deee09314f9eb86622 (diff) | |
| download | chef-263f62774641bf32be6fd79d1d69022531fb3285.tar.gz | |
Make FileVendor configuration specific to the two implementations
FileVendor previously was configured by storing a closure/anonymous
function as a class instance variable. This had the following downsides:
* The API was too general, which caused a lot of code repetition
* The block was lazily evaluated, which hid errors and made testing more
difficult
* The closures captured references to classes with references to large
data structures, which complicates GC.
Since we've only ever had the same two implementations of FileVendor, we
can encapsulate configuration of the FileVendor factory by wrapping each
configuration option in a method. As a side benefit, arguments to these
methods will be eagerly evaluated, which makes it easier to detect
errors.
Diffstat (limited to 'lib/chef/policy_builder')
| -rw-r--r-- | lib/chef/policy_builder/expand_node_object.rb | 4 | ||||
| -rw-r--r-- | lib/chef/policy_builder/policyfile.rb | 5 |
2 files changed, 3 insertions, 6 deletions
diff --git a/lib/chef/policy_builder/expand_node_object.rb b/lib/chef/policy_builder/expand_node_object.rb index 63964a2a0d..2ee8a23258 100644 --- a/lib/chef/policy_builder/expand_node_object.rb +++ b/lib/chef/policy_builder/expand_node_object.rb @@ -56,13 +56,13 @@ class Chef def setup_run_context(specific_recipes=nil) if Chef::Config[:solo] - Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::FileSystemFileVendor.new(manifest, Chef::Config[:cookbook_path]) } + Chef::Cookbook::FileVendor.fetch_from_disk(Chef::Config[:cookbook_path]) cl = Chef::CookbookLoader.new(Chef::Config[:cookbook_path]) cl.load_cookbooks cookbook_collection = Chef::CookbookCollection.new(cl) run_context = Chef::RunContext.new(node, cookbook_collection, @events) else - Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::RemoteFileVendor.new(manifest, api_service) } + Chef::Cookbook::FileVendor.fetch_from_remote(api_service) cookbook_hash = sync_cookbooks cookbook_collection = Chef::CookbookCollection.new(cookbook_hash) run_context = Chef::RunContext.new(node, cookbook_collection, @events) diff --git a/lib/chef/policy_builder/policyfile.rb b/lib/chef/policy_builder/policyfile.rb index 4baa6340d4..0df3dd5dd2 100644 --- a/lib/chef/policy_builder/policyfile.rb +++ b/lib/chef/policy_builder/policyfile.rb @@ -154,10 +154,7 @@ class Chef end def setup_run_context(specific_recipes=nil) - # TODO: This file vendor stuff is duplicated and initializing it with a - # block traps a reference to this object in a global context which will - # prevent it from getting GC'd. Simplify it. - Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::RemoteFileVendor.new(manifest, api_service) } + Chef::Cookbook::FileVendor.fetch_from_remote(http_api) sync_cookbooks cookbook_collection = Chef::CookbookCollection.new(cookbooks_to_sync) run_context = Chef::RunContext.new(node, cookbook_collection, events) |