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 core] Allow client to change driver/provider using HTTP headers
Date Thu, 09 Dec 2010 16:47:11 GMT
On 09/12/10 11:22 -0500, Toby Crawley wrote:
>Nice work. ACK, with comments below:

Thanks, I agree with all inline comments below.
Pushed to master.

   -- Michal

>
>On 12/09/2010 11:00 AM, mfojtik@redhat.com wrote:
>>From: Michal Fojtik<mfojtik@redhat.com>
>>
>>---
>>  client/lib/deltacloud.rb                 |   30 ++++++++++++++++++++++++------
>>  server/lib/sinatra/rack_driver_select.rb |    5 +----
>>  2 files changed, 25 insertions(+), 10 deletions(-)
>>
>>diff --git a/client/lib/deltacloud.rb b/client/lib/deltacloud.rb
>>index c632594..effd9bd 100644
>>--- a/client/lib/deltacloud.rb
>>+++ b/client/lib/deltacloud.rb
>>@@ -34,8 +34,9 @@ module DeltaCloud
>>    # @param [String, password] API password
>>    # @param [String, user_name] API URL (eg. http://localhost:3001/api)
>>    # @return [DeltaCloud::API]
>>-  def self.new(user_name, password, api_url,&block)
>>-    API.new(user_name, password, api_url,&block)
>>+  def self.new(user_name, password, api_url, opts={},&block)
>>+    opts ||= {}
>>+    API.new(user_name, password, api_url, opts,&block)
>>    end
>>
>>    # Check given credentials if their are valid against
>>@@ -62,11 +63,13 @@ module DeltaCloud
>>    end
>>
>>    class API
>>-    attr_reader   :api_uri, :driver_name, :api_version, :features, :entry_points
>>+    attr_reader :api_uri, :driver_name, :api_version, :features, :entry_points
>>+    attr_reader :api_driver, :api_provider
>>
>>      def initialize(user_name, password, api_url, opts={},&block)
>>        opts[:version] = true
>>-      @username, @password = user_name, password
>>+      @api_driver, @api_provider = opts[:driver], opts[:provider]
>>+      @username, @password = opts[:username] || user_name, opts[:password] || password
>>        @api_uri = URI.parse(api_url)
>>        @features, @entry_points = {}, {}
>>        @verbose = opts[:verbose] || false
>>@@ -241,6 +244,21 @@ module DeltaCloud
>>        raise NoMethodError
>>      end
>>
>>+    def use_driver(driver, opts={})
>>+      @api_driver = driver
>>+      @username = opts[:username]
>>+      @password = opts[:password]
>>+      @api_provider = opts[:provider] if opts[:provider]
>>+      return self
>>+    end
>>+
>
>I think it would be nice to be able to leave defaults, and just 
>override some of the options. That allows for a core that you know 
>just talks to ec2, and you just want to override the provider without 
>having to pass the driver and username/pw each call. Maybe instead of 
>the above use_driver, have instead:
>
>def use_driver(opts={})
>  @api_driver = opts[:driver] if opts[:driver]
>  @username = opts[:username] if opts[:username]
>  @password = opts[:password] if opts[:password]
>  @api_provider = opts[:provider] if opts[:provider]
>  self
>end
>
>This allows for:
>
>client = DeltaCloud.new('<aki>', '<sak>', '<core url>', :driver =>
:ec2)
>
>client.use_driver(:provider => 'us-west')
>
>#do stuff
>
>client.use_driver(:provider => 'us-east')
>
>
>>+    def extended_headers
>>+      headers = {}
>>+      headers["X-Deltacloud-Driver"] = "#{@api_driver}" if @api_driver
>>+      headers["X-Deltacloud-Provider"] = "#{@api_provider}" if @api_provider
>
>I would just use to_s here. To me, it reads better:
> headers["X-Deltacloud-Driver"] = @api_driver.to_s if @api_driver
>
>>+      headers
>>+    end
>>+
>>      # Basic request method
>>      #
>>      def request(*args,&block)
>>@@ -255,7 +273,7 @@ module DeltaCloud
>>        end
>>
>>        if conf[:method].eql?(:post)
>>-        RestClient.send(:post, conf[:path], conf[:form_data], default_headers) do
|response, request, block|
>>+        RestClient.send(:post, conf[:path], conf[:form_data], default_headers.merge(extended_headers))
do |response, request, block|
>>            handle_backend_error(response) if response.code.eql?(500)
>>            if response.respond_to?('body')
>>              yield response.body if block_given?
>>@@ -264,7 +282,7 @@ module DeltaCloud
>>            end
>>          end
>>        else
>>-        RestClient.send(conf[:method], conf[:path], default_headers) do |response,
request, block|
>>+        RestClient.send(conf[:method], conf[:path], default_headers.merge(extended_headers))
do |response, request, block|
>>            handle_backend_error(response) if response.code.eql?(500)
>>            if conf[:method].eql?(:get) and [301, 302, 307].include? response.code
>>              response.follow_redirection(request) do |response, request, block|
>>diff --git a/server/lib/sinatra/rack_driver_select.rb b/server/lib/sinatra/rack_driver_select.rb
>>index aa213d4..f00a2c8 100644
>>--- a/server/lib/sinatra/rack_driver_select.rb
>>+++ b/server/lib/sinatra/rack_driver_select.rb
>>@@ -15,10 +15,7 @@ class RackDriverSelect
>>    end
>>
>>    def extract_driver(env)
>>-    if env['HTTP_HEADERS']
>>-      driver_name = env['HTTP_HEADERS'].match(/X\-Deltacloud\-Driver:(\w+)/i).to_a
>>-      driver_name[1] ? driver_name[1].downcase : nil
>>-    end
>>+    driver_name = env['HTTP_X_DELTACLOUD_DRIVER'].downcase if env['HTTP_X_DELTACLOUD_DRIVER']
>>    end
>>
>>  end
>

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

Mime
View raw message