Mailing-List: contact deltacloud-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: deltacloud-dev@incubator.apache.org
Received-SPF: pass (nike.apache.org: domain of mfojtik@redhat.com designates
 209.132.183.28 as permitted sender)
Date: Mon, 15 Nov 2010 10:19:26 +0100
From: Michal Fojtik <mfojtik@redhat.com>
To: deltacloud-dev@incubator.apache.org
Subject: Re: [PATCH 2/2] Added actions to storage_volumes; changed 'size'
 to 'capacity' and 'zone' to 'realm_id' to match lingo elsewhere.
Message-ID: <20101115091926.GD3244@redhat.com>
References: <1289651571-8825-1-git-send-email-tcrawley@redhat.com>
 <1289651571-8825-3-git-send-email-tcrawley@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <1289651571-8825-3-git-send-email-tcrawley@redhat.com>
User-Agent: Mutt/1.5.20 (2009-06-14)

On 13/11/10 07:32 -0500, tcrawley@redhat.com wrote:
>From: Tobias Crawley <tcrawley@redhat.com>
>
>---
> server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |   15 +++++++++------
> .../lib/deltacloud/helpers/application_helper.rb   |    6 +++++-
> server/lib/deltacloud/models/storage_volume.rb     |    1 +
> server/server.rb                                   |    4 ++--
> server/views/storage_volumes/index.xml.haml        |   11 +----------
> server/views/storage_volumes/new.html.haml         |    4 ++--
> server/views/storage_volumes/show.xml.haml         |    9 ++++++++-
> 7 files changed, 28 insertions(+), 22 deletions(-)
>
>diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>index 14030a4..a0fd919 100644
>--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>@@ -281,14 +281,14 @@ module Deltacloud
>           end
>         end
>
>-        def create_storage_volume(credentials, opts={})
>+        def create_storage_volume(credentials, opts=nil)
>           ec2 = new_client(credentials)
>           opts ||= {}
>-          opts.merge!(:snapshot_id => "") unless opts[:snapshot_id]
>-          opts.merge!(:size => "1") unless opts[:volume_size]
>-          opts.merge!(:realm_id => realms(credentials).first.id) unless opts[:realm_id]
>+          opts[:snapshot_id] ||= ""
>+          opts[:capacity] ||= "1"
>+          opts[:realm_id] ||= realms(credentials).first.id
>           safely do
>-            convert_volume(ec2.create_volume(opts[:snapshot_id], opts[:size], opts[:realm_id]))
>+            convert_volume(ec2.create_volume(opts[:snapshot_id], opts[:capacity], opts[:realm_id]))

Agreeing with this change. Sound better to me.

>           end
>         end
>
>@@ -421,7 +421,10 @@ module Deltacloud
>             :capacity => volume[:aws_size],
>             :instance_id => volume[:aws_instance_id],
>             :realm_id => volume[:zone],
>-            :device => volume[:aws_device]
>+            :device => volume[:aws_device],
>+            # TODO: the available actions should be tied to the current
>+            # volume state
>+            :actions => [:attach, :detach, :destroy]
>           )
>         end
>
>diff --git a/server/lib/deltacloud/helpers/application_helper.rb b/server/lib/deltacloud/helpers/application_helper.rb
>index 00e8bc9..8f4db94 100644
>--- a/server/lib/deltacloud/helpers/application_helper.rb
>+++ b/server/lib/deltacloud/helpers/application_helper.rb
>@@ -38,9 +38,13 @@ module ApplicationHelper
>   end
>
>   def instance_action_method(action)
>-    collections[:instances].operations[action.to_sym].method
>+    action_method(action, :instances)
>   end
>
>+  def action_method(action, collection)
>+    collections[collection].operations[action.to_sym].method
>+  end
>+
>   def driver_has_feature?(feature_name, collection_name = :instances)
>     not driver.features(collection_name).select{ |f| f.name.eql?(feature_name) }.empty?
>   end
>diff --git a/server/lib/deltacloud/models/storage_volume.rb b/server/lib/deltacloud/models/storage_volume.rb
>index 2f379da..7870ef2 100644
>--- a/server/lib/deltacloud/models/storage_volume.rb
>+++ b/server/lib/deltacloud/models/storage_volume.rb
>@@ -25,5 +25,6 @@ class StorageVolume < BaseModel
>   attr_accessor :instance_id
>   attr_accessor :realm_id
>   attr_accessor :device
>+  attr_accessor :actions
>
> end
>diff --git a/server/server.rb b/server/server.rb
>index 424daab..5cab5ce 100644
>--- a/server/server.rb
>+++ b/server/server.rb
>@@ -450,8 +450,8 @@ collection :storage_volumes do
>     description "Create a new storage volume"
>     with_capability :create_storage_volume
>     param :snapshot_id, :string,  :optional
>-    param :size,        :string,  :optional
>-    param :zone,        :string,  :optional
>+    param :capacity,    :string,  :optional
>+    param :realm_id,    :string,  :optional
>     control do
>       @storage_volume = driver.create_storage_volume(credentials, params)
>       respond_to do |format|
>diff --git a/server/views/storage_volumes/index.xml.haml b/server/views/storage_volumes/index.xml.haml
>index 471169c..e73c46a 100644
>--- a/server/views/storage_volumes/index.xml.haml
>+++ b/server/views/storage_volumes/index.xml.haml
>@@ -1,13 +1,4 @@
> !!!XML
> %storage_volumes
>   - @elements.each do |volume|
>-    %storage_volume{ :href => storage_volume_url(volume.id), :id => volume.id }
>-      %created<
>-        =volume.created
>-      %capacity{ :unit => "GB" }<
>-        = volume.capacity
>-      - unless volume.instance_id.nil?
>-        %mount
>-          %instance{:href => instance_url(volume.instance_id), :id => volume.instance_id}
>-          - unless volume.device.nil?
>-            %device{ :name => volume.device }
>+    = haml :'storage_volumes/show', :locals =>  { :@storage_volume =>  volume, :partial =>  true }
>diff --git a/server/views/storage_volumes/new.html.haml b/server/views/storage_volumes/new.html.haml
>index 66bf015..f8ab6b8 100644
>--- a/server/views/storage_volumes/new.html.haml
>+++ b/server/views/storage_volumes/new.html.haml
>@@ -7,8 +7,8 @@
>     %input{ :name => 'snapshot_id', :size => 30 } (optional)
>   %p
>     %label
>-      Size (in GB):
>-    %input{ :name => "size", :size => 3, :value => "1"}
>+      Capacity (in GB):
>+    %input{ :name => "capacity", :size => 3, :value => "1"}
>   %p
>     %label
>       Realm ID:
>diff --git a/server/views/storage_volumes/show.xml.haml b/server/views/storage_volumes/show.xml.haml
>index ebf1b24..0a89192 100644
>--- a/server/views/storage_volumes/show.xml.haml
>+++ b/server/views/storage_volumes/show.xml.haml
>@@ -1,11 +1,18 @@
>-!!!XML
>+- unless defined?(partial)
>+  !!!XML
> %storage_volume{ :href => storage_volume_url(@storage_volume.id), :id => @storage_volume.id}
>   %created<
>     =@storage_volume.created
>   %capacity{ :unit => "GB" }<
>     = @storage_volume.capacity
>+  %state<
>+    = @storage_volume.state
>   - unless @storage_volume.instance_id.nil?
>     %mount
>       %instance{:href => @storage_volume.instance_id, :id => @storage_volume.instance_id}
>       - unless @storage_volume.device.nil?
>         %device{ :name => @storage_volume.device }
>+  - if @storage_volume.actions
>+    %actions
>+      - @storage_volume.actions.each do |action|
>+        %link{:rel => action, :method => action_method(action, :storage_volumes), :href => self.send("#{action}_storage_volume_url", @storage_volume.id)}
>--
>1.7.2.3
>

Great work Tobi. ACK!

Also I will apply your patch on top of my branch and continue work with
this patches applied. So the rev2 will have all your patches.

   -- Michal

-- 
--------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
--------------------------------------------------------