You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by ma...@redhat.com on 2011/02/03 21:30:08 UTC
Rework Buckets GET method
GET /api/buckets
previously, this would request details for *all buckets* - I realise now that this is not workable, especially when you have a large number of buckets. Instead, will only get details (i.e. list of blobs) if a specific bucket is requested (GET /api/buckets/:bucket) and otherwise will only report a list of bucket names.
Many thanks to Jan Provaznik for pointing at this (and subsequently poking Michal with a sharp stick),
marios
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
Posted by Michal Fojtik <mf...@redhat.com>.
On 03/02/11 22:30 +0200, marios@redhat.com wrote:
ACK. Just minor inline comments.
-- Michal
>From: marios <ma...@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
--------------------------------------------------------
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
Posted by David Lutterkort <lu...@redhat.com>.
On Thu, 2011-02-03 at 22:30 +0200, marios@redhat.com wrote:
> From: marios <ma...@redhat.com>
ACK from me too. Just a minor nitpick to prove I read the patch: ;)
> --- 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
I personally dislike marking end's that way. If they are truly needed to
find your way through the code, it's a strong hint that the code should
be broken up into smaller routines (though I don't think that that's the
case here)
David
[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
Posted by ma...@redhat.com.
From: marios <ma...@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 = {} )
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})
+ 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
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)
- 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)
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