Re: [PATCH core] Allow client to change driver/provider using HTTP headers

[Toby Crawley]

Nice work. ACK, with comments below:

On 12/09/2010 11:00 AM, mfoj...@redhat.com wrote:
From: Michal Fojtik<mfoj...@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