Issue-15 should be able to pass in cfgfile

[bradbeam]

There's a somewhat related but hidden addition with this.. I can split it out into a separate PR if necessary.

I updated the chef_config.sh to create the .chef directory local to the project as to not interfere with existing knife workstation setups and let the tests truly be isolated. I've included the .chef directory, but we may want to pull it out and add it to a .gitignore to keep the project clean.

[spheromak]

@bradbeam re: chef_config.sh it's just there for CI i wouln't expect a developer to use it so much, when we can pass a path to the config file it will not be needed anymore.

It's late here so ill check out the actual PR in the morning.

[marpaia]

Thanks for tackling this! Would you mind breaking this into two commits? One where you move existing functionality from api.go to config_parser.go and another where you extend the code and add new functionality?

[bradbeam]

Will do.

I also was starting to play around some more and noticed my knife.rb is

$ cat ~/.chef/knife.rb
current_dir = File.dirname(__FILE__)
log_level                :info
log_location             STDOUT
node_name                "bradbeam"
client_key               "#{current_dir}/bradbeam - chef.com.pem"
validation_client_name   "main-validator"
validation_key           "#{current_dir}/main-validator.pem"
chef_server_url          "https://chef.com/organizations/main"
cache_type               'BasicFile'
cache_options( :path => "#{ENV['HOME']}/.chef/checksums" )
cookbook_path            ["#{current_dir}/../cookbooks"]

So we'll probably need to extend the functionality to interpret / replace the embedded ruby

[spheromak]

@bradbeam yep. Hello mruby to do that. which adds ext dep (and a complex one at that). This is why i wanted the parsing separate from the config package.

I feel like the config parser isn't what this API is for. If you were making a chef-client you would build out a parser for the client.rb. That would bring it into a Config type. Letting the API only deal with that, and building out another project for actually parsing chef/knife/ruby config. Then in your tool you would import from chef-golang, and the parser package.

Interested in thoughts from @marpaia on that as well. I am against trying to interpret the ruby. In this project at least.

[marpaia]

@spheromak, I see your logic. I think that we, theoretically, should be able to parse any config file that the ruby chef API client can. Even if that means that we expose a function that interfaces with a different parsing package, I could totally see it being annoying to users if we supported everything except config file parsing.

with that being said, I think supporting embedded ruby is too far down the rabbit hole. perhaps that's where we draw the line?

[bradbeam]

Holy regex batman! :dancer:

[marpaia]

I'm going away for the weekend, I'll defer to @spheromak.

[spheromak]

@bradbeam can we close this one out. replaced by #25 ?

[bradbeam]

We can close either one, I misread one of the previous comments and thought we wanted to do two PR's for it. If we want to keep this one for the discussion trail I'm fine with that and pushing the updates here.

re: keyfromstring/file, those should be addressed in 4ef5700 on #24 but I forgot to apply to #25

re: uglyness, Yeah I was hoping for a much more elegant solution, but between cookbook_path [1] being a ruby array and no_proxy[2] being a string of comma separated values things started to get more complicated.

re: config vs knifeconfig, how do we scope what attributes we care about? Do we need the attributes exposed in client.rb instead?

[1] https://github.com/marpaia/chef-golang/pull/25/files#diff-fa9cc37a4f73197609b7fbc0a0ba84f8R103

[2] https://github.com/marpaia/chef-golang/pull/25/files#diff-fa9cc37a4f73197609b7fbc0a0ba84f8R120

[spheromak]

@bradbeam I think @marpaia wanted to pull out the 2 logical commits. what mods api and what adds the config parser. If i recall right. I am ok with one PR, lets just get one that we can comment/work against :)

re: scope. This isn't a config parsing library its a Server connection library. IMO we should take a Config struct that is what we need to connect to the server and do what we need. chef-config-golang should be a different project IMO.

re: complexity yea i get it. To do it right we would need to lex/parse, and thats out of scope for this api package.

P.S. thanks for the work on this.

[bradbeam]

Started looking more into this and I'm curious on the thoughts around chef.Version. From http://docs.opscode.com/api_chef_server.html, it sounds like X-Chef-Version is pretty heavily tied to the client. Do we want to allow this to be custom input?

Going to give it another go and see what happens from there :japanese_ogre:

[bradbeam]

I was thinking more about it and decided to keep this limited to just being able to pass in a config file.

I know there have been some suggestions on breaking it out further and in looking at the various connect methods and I think there are some opportunities ( #16 ) to clean up the various api connect methods. Does that sound fair?

[spheromak]

thanks @bradbeam this looks like the right minimal change.