-
Notifications
You must be signed in to change notification settings - Fork 450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove XML output in VirtualizationInfo and need for hpricot gem #755
Conversation
This is an early plugin that hasn't received any work since it's creation. I seriously doubt anyone is using it, but this provides some minor cleanup 1) remove the hpricot dependency and all the commented out hpricot code. This plugin would fail unless it was installed, but it's not actually used 2) Stop storing XML data on the node. This is a horrible horrible thing to do. We're storing the exact same data in JSON. Why 2 formats? 3) Change the debug message to make it clean what plugin fails if the gems aren't there 4) Add a comment telling anyone reading this what gem to use. The libvirt gem is deprecated and fails to load. It's a bummer we need this gem to use this because the data is actually pretty useful once it's loaded, but I doubt anyone installs the gem.
|
@chef/client-core this is ready for review |
presumably the gem has native code dependencies? Or does it talk to the libvirt daemon? |
@thommay It's just a nice way to interfact with libvirt daemon. It's not worth bundling and I think the way we're storing the output is wrong. Unfortunately we screwed up the way we store virtualization information when we added systems and missed a great opportunity to properly nest data under the virtualization mash. This data would be really great if we could store it somewhere like this.
|
👍 |
1 similar comment
👍 |
Remove XML output in VirtualizationInfo and need for hpricot gem
This is an early plugin that hasn't received any work since it's creation. I seriously doubt anyone is using it, but this provides some minor cleanup
It's a bummer we need this gem to use this because the data is actually pretty useful once it's loaded, but I doubt anyone installs the gem.