Hi Martha,

I agree with Michal's ACk, though there's one small nit:

On Tue, 2013-02-05 at 10:45 +0300, [email protected] wrote:
> diff --git a/clients/cimi/lib/entities/machine_template.rb 
> b/clients/cimi/lib/entities/machine_template.rb
> index ccc80a3..026d244 100644
> --- a/clients/cimi/lib/entities/machine_template.rb
> +++ b/clients/cimi/lib/entities/machine_template.rb
> @@ -22,9 +22,33 @@ class CIMI::Frontend::MachineTemplate < 
> CIMI::Frontend::Entity
>    end
...
> +  post '/cimi/machine_templates' do
> +    machine_template_xml = Nokogiri::XML::Builder.new do |xml|
> +      xml.MachineTemplate(:xmlns => CIMI::Frontend::CMWG_NAMESPACE) {
> +        xml.name params[:machine_template][:name]
> +        xml.description params[:machine_template][:description]
> +        xml.machineConfig( :href => 
> params[:machine_template][:machine_configuration] )
> +        xml.machineImage( :href => params[:machine_template][:machine_image] 
> )
> +      }
> +    end.to_xml
> +    begin
> +      result = create_entity('machine_templates', machine_template_xml, 
> credentials)
> +      machine_template = 
> CIMI::Model::MachineTemplateCollection.from_xml(result)
> +      flash[:success] = "Machine Template was successfully created."
> +      redirect "/cimi/machine_templates/#{machine_template.name}", 302

Here you're explicitly constructing a URL, which means that this code
will only work with a CIMI server that has the same URL structure as
Deltacloud. I know that the CIMI webapp does that all over the place,
but we need to get rid of that.

The more CIMI compliant way to do this is to get the 'id' attribute from
result, i.e. something along the lines of 'redirect
JSON::parse(result.body)["id"] 302'

Have a look at how RestClient::Response is monkey-patched in
tests/helpers/common.rb to see how you could write this as 'redirect
result.json["id"] 302' by pulling the monkey-patch into your client.rb

David


Reply via email to