Hi,
I added a new feature named 'brief_listing_mode' which fixing a Terremark issue (and maybe other providers) when a full objects are returned in 'index' action. This can can be a long and slow process, leading to random HTTP timeouts for clients.
So instead of returning a full object, only URL and id is returned for object. Details can be obtained using GET method.
Because it's implemented as a 'feature', it doesn't break API for drivers, which don't have this feature declared.
Also I added an option to return a full object using '&listing_mode=full'.
I enabled this feature for Mock driver for testing supposes and I'll remove it once this patch will be approved.
To try it out, just start API using Mock driver and:
http://localhost:3001/api/images.xml http://localhost:3001/api/images.xml?listing_mode=full http://localhost:3001/api/instances.xml http://localhost:3001/api/instances.xml?listing_mode=full
-- Michal
--- server/lib/deltacloud/base_driver/features.rb | 12 +++++ server/lib/deltacloud/drivers/mock/mock_driver.rb | 2 + .../lib/deltacloud/helpers/application_helper.rb | 12 +++++- server/views/images/index.xml.haml | 11 +++-- server/views/instances/index.xml.haml | 43 ++++++++++--------- 5 files changed, 54 insertions(+), 26 deletions(-)
diff --git a/server/lib/deltacloud/base_driver/features.rb b/server/lib/deltacloud/base_driver/features.rb index 8ad354a..39f0a94 100644 --- a/server/lib/deltacloud/base_driver/features.rb +++ b/server/lib/deltacloud/base_driver/features.rb @@ -151,6 +151,18 @@ module Deltacloud end end
+ declare_feature :images, :brief_listing_mode do + operation :index do + param :listing_mode, :string, :optional + end + end + + declare_feature :instances, :brief_listing_mode do + operation :index do + param :listing_mode, :string, :optional + end + end + declare_feature :instances, :hardware_profiles do description "Size instances according to changes to a hardware profile" # The parameters are filled in from the hardware profiles diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb b/server/lib/deltacloud/drivers/mock/mock_driver.rb index 9671f02..3096cd6 100644 --- a/server/lib/deltacloud/drivers/mock/mock_driver.rb +++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb @@ -75,6 +75,8 @@ class MockDriver < Deltacloud::BaseDriver end
feature :instances, :user_name + feature :images, :brief_listing_mode + feature :instances, :brief_listing_mode
def initialize if ENV["DELTACLOUD_MOCK_STORAGE"] diff --git a/server/lib/deltacloud/helpers/application_helper.rb b/server/lib/deltacloud/helpers/application_helper.rb index 850fab4..1428ec0 100644 --- a/server/lib/deltacloud/helpers/application_helper.rb +++ b/server/lib/deltacloud/helpers/application_helper.rb @@ -40,8 +40,8 @@ module ApplicationHelper collections[:instances].operations[action.to_sym].method end
- def driver_has_feature?(feature_name) - not driver.features(:instances).select{ |f| f.name.eql?(feature_name) }.empty? + def driver_has_feature?(feature_name, default_collection = :instances) + not driver.features(default_collection).select{ |f| f.name.eql?(feature_name) }.empty? end
def driver_has_auth_features? @@ -53,4 +53,12 @@ module ApplicationHelper return 'password' if driver_has_feature?(:authentication_password) end
+ def full_listing_mode?(collection) + if driver_has_feature?(:brief_listing_mode, collection) and not params[:listing_mode]=='full' + return false + else + return true + end + end + end diff --git a/server/views/images/index.xml.haml b/server/views/images/index.xml.haml index 6d14b21..a86cc5b 100644 --- a/server/views/images/index.xml.haml +++ b/server/views/images/index.xml.haml @@ -1,7 +1,10 @@ !!! XML %images - @elements.each do |image| - %image{:href => image_url(image.id)} - - image.attributes.each do |attribute| - - haml_tag(attribute, :<) do - - haml_concat image.send(attribute) + - if full_listing_mode?(:images) + %image{:href => image_url(image.id)} + - image.attributes.each do |attribute| + - haml_tag(attribute, :<) do + - haml_concat image.send(attribute) + - else + %image{ :href => image_url(image.id), :id => image.id } diff --git a/server/views/instances/index.xml.haml b/server/views/instances/index.xml.haml index f2bb8f0..176d706 100644 --- a/server/views/instances/index.xml.haml +++ b/server/views/instances/index.xml.haml @@ -1,23 +1,26 @@ !!! XML %instances - @elements.each do |instance| - %instance{:href => instance_url(instance.id)} - %id #{instance.id} - %name #{instance.name} - %owner_id #{instance.owner_id} - %image{:href => image_url(instance.image_id)} - %realm{:href => realm_url(instance.realm_id)} - %state #{instance.state} - - haml_tag :"hardware-profile", { :href => hardware_profile_url(instance.instance_profile.id)} do - %id #{instance.instance_profile.id} - - instance.instance_profile.overrides.each do |p, v| - %property{:kind => 'fixed', :name => p, :value => v, :unit => Deltacloud::HardwareProfile::unit(p)} - %actions - - instance.actions.compact.each do |action| - %link{:rel => action, :href => self.send("#{action}_instance_url", instance.id), :method => instance_action_method(action)} - %public-addresses - - instance.public_addresses.each do |address| - %address #{address} - %private-addresses - - instance.private_addresses.each do |address| - %address #{address} + - if full_listing_mode?(:instances) + %instance{:href => instance_url(instance.id)} + %id #{instance.id} + %name #{instance.name} + %owner_id #{instance.owner_id} + %image{:href => image_url(instance.image_id)} + %realm{:href => realm_url(instance.realm_id)} + %state #{instance.state} + - haml_tag :"hardware-profile", { :href => hardware_profile_url(instance.instance_profile.id)} do + %id #{instance.instance_profile.id} + - instance.instance_profile.overrides.each do |p, v| + %property{:kind => 'fixed', :name => p, :value => v, :unit => Deltacloud::HardwareProfile::unit(p)} + %actions + - instance.actions.compact.each do |action| + %link{:rel => action, :href => self.send("#{action}_instance_url", instance.id), :method => instance_action_method(action)} + %public-addresses + - instance.public_addresses.each do |address| + %address #{address} + %private-addresses + - instance.private_addresses.each do |address| + %address #{address} + - else + %instance{ :id => instance.id, :href => instance_url(instance.id)}
On Mon, 2010-05-31 at 18:10 +0200, mfojtik@redhat.com wrote:
I added a new feature named 'brief_listing_mode' which fixing a Terremark issue (and maybe other providers) when a full objects are returned in 'index' action. This can can be a long and slow process, leading to random HTTP timeouts for clients.
So instead of returning a full object, only URL and id is returned for object. Details can be obtained using GET method.
Because it's implemented as a 'feature', it doesn't break API for drivers, which don't have this feature declared.
There's not really any need to make this a feature - clients can tell from the response to the index operation if they are getting a full or brief listing.
To make their life easier, we could add an attribute on the enclosing <instances/> or <images/> tag, something like <instances listing='brief'>; in the ruby client, we should support this by making objects lazy-load their attributes (IIRC, there's already some support for that)
The behavior should be that we return a full listing from drives that make that cheap, i.e. allow listing with just one call into their API, and return a brief listing from all other drivers.
Also I added an option to return a full object using '&listing_mode=full'.
I don't think this is very useful - for drivers that make full listings cheap, we already send them. For the others, a full listing is just asking for trouble and random behavior, based on whether we'll hit a timeout or not.
David
deltacloud-devel@lists.fedorahosted.org