Closed brettswift closed 10 years ago
Hi @brettswift, thank you so much for this PR. It looks good, could you just add a deprecation warning at log level (see set_vm_network_config)?
I left the test for one deprecated method, but can remove it...
Ok perfect! I think I'll squash those two commits. You used two different mail addresses, which one would you like to keep?
Gmail one please. I was going to re-write history but got a bit nervous with how that would affect the PR.
Thanks !
Brett Sent from my iPhone
On Jan 3, 2014, at 9:53, Stefano Tortarolo notifications@github.com wrote:
Ok perfect! I think I'll squash those two commits. You used two different mail addresses, which one would you like to keep?
— Reply to this email directly or view it on GitHub.
Ok, squashed and merged: https://github.com/astratto/vcloud-rest/commit/226c6addd31cadacf0837b0282a5b37946cdc334
Thank you again!
Hey @astratto,
The API documentation is very obscure about this, but you can actually snapshot a VM. It's just unintuitive because your request is sent to the ./vapp/ URI. You do a ./vapp/_vm_-{id}/action..... for vm's and ./vapp/_vapp_-{id} for vapp's.
Because of this behaviour, I pulled the snapshot behaviour out of vapp, and into connection.rb. I kept the old method, but the old vapp method is now commented as deprecated, and delegates to the connection.rb.
vapp/create_snapshot(id) --> connection/create_vapp_snapshot(id, :vapp) vapp/create_vapp_snapshot(id) --> connection/create_vapp_snapshot(id, :vapp) vm/create_vm_snapshot(id) --> connection/create_vm_snapshot(id, :vm)
As you can see I used the same pattern you had for the power_actions common method.
I want to include mocha, and do an assertion unit test for when create_snapshot_action (couldn't come up with a better name) is called that the :type property is being used properly. So I can put that in before this PR is merged.
Please comment on implementation - if you'd rather see it implemented a different way - I'd be interested to hear!
Thanks!