Hi, Sure, I'll rebase and split this to smaller commits with proper message. Thanks for pointing this out :-)
-- Michal Michal Fojtik http://deltacloud.org [email protected] On Feb 22, 2012, at 2:03 AM, David Lutterkort wrote: > On Tue, 2012-02-21 at 15:21 +0100, [email protected] wrote: >> From: Michal Fojtik <[email protected]> > > Why does the commit message say 'RHEV-M' ? The patch doesn't touch the > RHEV-M driver. 'CIMI' seems more appropriate. > > Couple more comments: > >> diff --git a/clients/cimi/lib/entities/machine.rb >> b/clients/cimi/lib/entities/machine.rb >> index d70d0e9..c17925b 100644 >> --- a/clients/cimi/lib/entities/machine.rb >> +++ b/clients/cimi/lib/entities/machine.rb >> @@ -86,10 +91,10 @@ class CIMI::Frontend::Machine < CIMI::Frontend::Entity >> xml.Machine(:xmlns => CIMI::Frontend::CMWG_NAMESPACE) { >> xml.name params[:machine][:name] >> xml.description params[:machine][:description] >> - xml.MachineTemplate { >> - xml.MachineConfig( :href => >> params[:machine][:machine_configuration] ) >> - xml.MachineImage( :href => params[:machine][:machine_image] ) >> - xml.MachineAdmin( :href => params[:machine][:machine_admin] ) >> unless params[:machine][:machine_admin].empty? >> + xml.machineTemplate { >> + xml.machineConfig( :href => >> params[:machine][:machine_configuration] ) >> + xml.machineImage( :href => params[:machine][:machine_image] ) >> + xml.machineAdmin( :href => params[:machine][:machine_admin] ) >> unless params[:machine][:machine_admin].nil? > > This hunk doesn't seem to have anything to do with exception handling; > should really go into its own patch. (And then I wouldn't have to stare > at it to figure out it's about changing the capitalization of tags; > still don't know why the change from empty? to nil?) > >> diff --git a/server/lib/cimi/model/machine.rb >> b/server/lib/cimi/model/machine.rb >> index 949a072..88c9f73 100644 >> --- a/server/lib/cimi/model/machine.rb >> +++ b/server/lib/cimi/model/machine.rb >> @@ -82,6 +82,7 @@ class CIMI::Model::Machine < CIMI::Model::Base >> hardware_profile_id = >> machine_template['machineConfig'][0]["href"].split('/').last >> image_id = machine_template['machineImage'][0]["href"].split('/').last >> additional_params = {} >> + additional_params[:name] =xml['name'][0] if xml['name'] > > This also doesn't seem to be related to MachineAdmin (the reason I harp > on this is that when I write the NEWS for the next release, changes like > this just disappear) > >> diff --git a/server/lib/deltacloud/backend_capability.rb >> b/server/lib/deltacloud/backend_capability.rb >> index 57e8cd3..05e193c 100644 >> --- a/server/lib/deltacloud/backend_capability.rb >> +++ b/server/lib/deltacloud/backend_capability.rb >> @@ -37,7 +37,8 @@ module Deltacloud::BackendCapability >> >> def check_capability(backend) >> if !has_capability?(backend) >> - raise Failure.new(capability, "#{capability} capability not supported >> by backend #{backend.class.name}") >> + e = Failure.new(capability, "#{capability} capability not supported >> by backend #{backend.class.name}") >> + raise Deltacloud::ExceptionHandler::NotSupported.new(e, nil) >> end >> end >> >> diff --git a/server/lib/deltacloud/base_driver/exceptions.rb >> b/server/lib/deltacloud/base_driver/exceptions.rb >> index e44c89b..3969e7e 100644 >> --- a/server/lib/deltacloud/base_driver/exceptions.rb >> +++ b/server/lib/deltacloud/base_driver/exceptions.rb >> @@ -79,6 +79,13 @@ module Deltacloud >> end >> end >> >> + class NotSupported < DeltacloudException >> + def initialize(e, message) >> + message ||= e.message >> + super(501, e.class.name, message, e.backtrace) >> + end >> + end > > Why create the Failure exception, just to take it apart and not use it ? > > David > >
