deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Lutterkort <lut...@redhat.com>
Subject Re: [PATCH 1/3] post method to machine_template
Date Tue, 05 Feb 2013 19:08:18 GMT
Hi Martha,

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

On Tue, 2013-02-05 at 10:45 +0300, martha.c.chumo@gmail.com 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



Mime
View raw message