incubator-deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Fojtik <mfoj...@redhat.com>
Subject Re: [PATCH] Reworks GET BUCKET so that only get a full listing of blobs if a specific bucket is requested (and just list of bucket names otherwise) - azure, ec2, cloudfiles
Date Fri, 04 Feb 2011 09:50:41 GMT
On 03/02/11 22:30 +0200, marios@redhat.com wrote:

ACK. Just minor inline comments.

   -- Michal

>From: marios <marios@redhat.com>
>
>---
> .../lib/deltacloud/drivers/azure/azure_driver.rb   |   13 ++++++++---
> server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |   22 +++++++++++--------
> .../drivers/rackspace/rackspace_driver.rb          |   13 +++++++----
> .../lib/deltacloud/helpers/application_helper.rb   |    2 +-
> server/views/buckets/index.html.haml               |   13 +----------
> server/views/buckets/index.xml.haml                |    8 ++----
> 6 files changed, 35 insertions(+), 36 deletions(-)
>
>diff --git a/server/lib/deltacloud/drivers/azure/azure_driver.rb b/server/lib/deltacloud/drivers/azure/azure_driver.rb
>index f17e11e..0bbf5b0 100644
>--- a/server/lib/deltacloud/drivers/azure/azure_driver.rb
>+++ b/server/lib/deltacloud/drivers/azure/azure_driver.rb
>@@ -36,10 +36,15 @@ class AzureDriver < Deltacloud::BaseDriver
>     buckets = []
>     azure_connect(credentials)
>     safely do
>-      WAZ::Blobs::Container.list.each do |waz_container|
>-        buckets << convert_container(waz_container)
>-      end
>-    end
>+      unless (opts[:id].nil?)
>+        waz_bucket =  WAZ::Blobs::Container.find(opts[:id])
>+        buckets << convert_container(waz_bucket)
>+      else
>+        WAZ::Blobs::Container.list.each do |waz_container|
>+          buckets << Bucket.new({:id =>waz_container.name, :name => waz_container.name})
>+        end #container.list.each
>+      end #unless
>+    end #safely
>     buckets = filter_on(buckets, :id, opts)
>   end
>
>diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>index 7436fb8..8b4d435 100644
>--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>@@ -323,16 +323,20 @@ module Deltacloud
>           end
>         end
>
>-
>-        def buckets(credentials, opts={})
>+        def buckets(credentials, opts = {} )

Not sure if this change s needed ;-))

>           buckets = []
>           safely do
>             s3_client = new_client(credentials, :s3)
>-            bucket_list = s3_client.buckets
>-            bucket_list.each do |current|
>-              buckets << convert_bucket(current)
>-            end
>-          end
>+            unless (opts[:id].nil?)
>+              bucket = s3_client.bucket(opts[:id])
>+              buckets << convert_bucket(bucket)
>+            else
>+              bucket_list = s3_client.buckets
>+              bucket_list.each do |current|
>+                buckets << Bucket.new({:name => current.name, :id => current.name})

You can safely omit {} here in future. If the last or only argument is
Hash, you can do: .new(:name => '', :id => ''). But this is not a blocker
of course ;-)

>+              end #bucket_list.each
>+            end #if
>+          end #safely
>           filter_on(buckets, :id, opts)
>         end
>
>@@ -574,13 +578,13 @@ module Deltacloud
>           #get blob list:
>           blob_list = []
>           s3_bucket.keys.each do |s3_object|
>-            blob_list << s3_object.name
>+              blob_list << s3_object.name

Tab above is not needed as well.

>           end
>           #can use AWS::S3::Owner.current.display_name or current.id
>           Bucket.new(
>             :id => s3_bucket.name,
>             :name => s3_bucket.name,
>-            :size => s3_bucket.keys.length,
>+            :size => blob_list.length,
>             :blob_list => blob_list
>           )
>         end
>diff --git a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
>index eba8f8f..0887a56 100644
>--- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
>+++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
>@@ -170,13 +170,16 @@ class RackspaceDriver < Deltacloud::BaseDriver
>     bucket_list = []
>     cf = cloudfiles_client(credentials)
>     safely do
>-      cf.containers.each do |container_name|
>-        current = cf.container(container_name)
>-        bucket_list << convert_container(current)
>-      end #containers.each
>+      unless (opts[:id].nil?)
>+        bucket = cf.container(opts[:id])
>+        bucket_list << convert_container(bucket)
>+      else
>+        cf.containers.each do |container_name|
>+          bucket_list << Bucket.new({:id => container_name, :name => container_name})
>+        end #containers.each
>+      end #unless
>     end #safely
>     bucket_list = filter_on(bucket_list, :id, opts)

You can omit 'bucket_list =' here, since filter_on will return it.

>-    bucket_list
>   end
>
> #--
>diff --git a/server/lib/deltacloud/helpers/application_helper.rb b/server/lib/deltacloud/helpers/application_helper.rb
>index abe78bc..f1909b2 100644
>--- a/server/lib/deltacloud/helpers/application_helper.rb
>+++ b/server/lib/deltacloud/helpers/application_helper.rb
>@@ -76,7 +76,7 @@ module ApplicationHelper
>       filter.merge!(:architecture => params[:architecture]) if params[:architecture]
>       filter.merge!(:owner_id => params[:owner_id]) if params[:owner_id]
>       filter.merge!(:state => params[:state]) if params[:state]
>-      filter = nil if filter.keys.size.eql?(0)
>+      filter = {} if filter.keys.size.eql?(0)

This is nice catch! I was going to ask you all about this one. I'm not sure
if our second argument in collection/item is Hash or String. So:

instance(credentials, id) OR instance(credentials, :id => 1)

Seems like second one is correct. I'll check all drivers and method to
ensure that we use just one apporach for this to avoid future arg
exceptions.

>       singular = model.to_s.singularize.to_sym
>       @benchmark = Benchmark.measure do
>         @elements = driver.send(model.to_sym, credentials, filter)
>diff --git a/server/views/buckets/index.html.haml b/server/views/buckets/index.html.haml
>index 0cb7ebe..5acc5f6 100644
>--- a/server/views/buckets/index.html.haml
>+++ b/server/views/buckets/index.html.haml
>@@ -11,11 +11,6 @@
>         ID
>       %th
>         Name
>-      %th
>-        Size
>-      %th
>-        Blob List
>-      %th
>
>   %tbody
>     - @buckets.each do |bucket|
>@@ -25,10 +20,4 @@
>         %td
>           = bucket.name
>         %td
>-          = bucket.size
>-        %td
>-          -bucket.blob_list.each do |blob|
>-            = blob
>-        %td
>-          -if bucket.size == 0
>-            =link_to_action 'Delete', destroy_bucket_url(bucket.name), :delete
>+          =link_to_action 'Delete', destroy_bucket_url(bucket.name), :delete
>diff --git a/server/views/buckets/index.xml.haml b/server/views/buckets/index.xml.haml
>index a81432c..e8cbeec 100644
>--- a/server/views/buckets/index.xml.haml
>+++ b/server/views/buckets/index.xml.haml
>@@ -3,8 +3,6 @@
>   - @elements.each do |bucket|
>     %bucket{:href => bucket_url(bucket.id), :id => bucket.id}
>       - bucket.attributes.select{ |attr| attr!=:id }.each do |attribute|
>-        - unless attribute == :blob_list
>-          - haml_tag("#{attribute}".tr('-', '_'), :<) do
>-            - haml_concat bucket.send(attribute)
>-      - bucket.blob_list.each do |blb|
>-        %blob{ :id => blb, :href => bucket_url(bucket.id) +"/#{blb}"}
>+        - haml_tag("#{attribute}".tr('-', '_'), :<) do
>+          - haml_concat bucket.send(attribute)
>+
>--
>1.7.3.4
>

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

Mime
View raw message