Closed nanliu closed 9 years ago
@adrienthebo any decision on this yet? I need to decide if I should fork this plugin and release as a new gem or wait till this is merged.
@logicminds vagrant-config_builder itself is pluggable, so if you want to use a plugin that's not in core you can add it yourself at runtime. vagrant-hosts implements the model at https://github.com/adrienthebo/vagrant-hosts/blob/master/lib/vagrant-hosts/config_builder.rb and will selectively load the model if and only if the config_builder plugin is present. If you want a quick and dirty version you can skip this and get it working right now by just moving that to the Vagrantfile:
# Vagrantfile
require 'config_builder'
# Vagrant Trigger model
#
# @see https://github.com/emyl/vagrant-triggers
class ConfigBuilder::Model::Trigger < ConfigBuilder::Model::Base
# @!attribute [rw] trigger
# @return [String] The name of the trigger (before, after, instead_of)
def_model_attribute :trigger
# @!attribute [rw] command
# @return [String] The vagrant command
def_model_attribute :command
# @!attribute [rw] host
# @return [Hash] The trigger options
def_model_attribute :options
# @!attribute [rw] script
# @return [String] The script
def_model_attribute :script
def to_proc
script = attr(:script)
Proc.new do |config|
config.trigger.send(attr(:trigger).to_sym, attr(:command).to_sym, attr(:options)) do
eval script
end
end
end
end
class ConfigBuilder::Model::Root
def_model_delegator :triggers
def eval_triggers(root_config)
triggers = attr(:triggers) || []
triggers.each do |config|
f = ConfigBuilder::Model::Trigger.new_from_hash(config)
f.call(root_config)
end
end
end
Vagrant.configure('2', &ConfigBuilder.load(
:yaml_erb,
:yamldir,
File.expand_path('../config', __FILE__)
))
It's a bit hack to drop it into the Vagrantfile, but it works.
As the vagrant-hosts example demonstrates, if you want a config_builder plugin you can wrap that up in a gem and publish that without having to fork and maintain a copy of the entire config_builder gem.
Anywho, so let's talk about this pull request itself.
So the reason I wrote this plugin in the first place is to avoid having to write Ruby in order to define VMs, and flipping that around and writing Ruby inside of YAML seems like crazy town. I'm happy to be convinced otherwise of course, and if this sort of thing is reasonable to do and won't end up with people writing large Ruby scripts inside of YAML that's loaded by a Ruby DSL, then fine. So will this help people in the long run or will this lead people to shooting themselves in the foot?
And to emphasize, config_builder is meant to be pluggable itself so that you can extend things as needed without having to keep every single model in the main repo. There's another example, vagrant-pe_build which does the same thing (see https://github.com/adrienthebo/vagrant-pe_build/blob/master/lib/pe_build/plugin.rb#L102-L104 , https://github.com/adrienthebo/vagrant-pe_build/blob/master/lib/pe_build/config_builder/global.rb, and https://github.com/adrienthebo/vagrant-pe_build/blob/master/lib/pe_build/config_builder/pe_bootstrap.rb). So if you do want to fork the entire project then that's totally your call, but it's not necessary to fork the project in order to add your own extensions.
This is excellent information. I was not aware I could just write a plugin in order to add additional code to config_builder. I think that may be the best route.
Let me do some research on this topic and mock up a quick plugin gem to ensure everything works. If this works I don't see a reason to merge this code into core unless tons of people start using the feature.
As far as having Ruby in Yaml portion. It is crazy, but having the ability to use triggers that call ruby procs is nice when I have to respond to vagrant events. And we are not using ruby to define the vm, instead we are using ruby to respond to a vagrant event. It just so happens the config builder DSL is in yaml and putting ruby in yaml will always be ugly.
Thanks for getting back and all the hard work you have saved me thus far by creating this gem.
For what it's worth, config_builder doesn't need to use YAML as the data source. The architecture of config_builder is this:
Loader (Loads information, produces a hash) | v Filters (produces a hash, modifies that hash, returns the hash) | v Model (Takes a hash, delegates data as needed, turns it all into Vagrant config)
YAML is just the Loader that I happened to implement, but you can use any conceivable data source - you could use JSON, TOML, another Ruby DSL if that floats your boat, just something that produces a hash. In the same manner that you can plug in new Models, you can plug in new loaders: https://github.com/adrienthebo/vagrant-config_builder/blob/master/lib/config_builder/loader/yaml.rb#L43
So if you don't want to write Ruby in YAML and you've got a better way to express your data than YAML, you can use whatever loader you want. If that loader can produce Ruby procs, then instead of doing eval ruby
you can do ruby.call()
.
Sorry, took a while to loop back to this PR. The comments have been very helpful. Thanks for all the suggestions.
@adrienthebo Is there any way we can get this merged? I am finding this PR useful.