You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2012/05/22 23:38:48 UTC

Unifying driver method parameters across all drivers

Hi,

I created a minitest that check consistency of the driver methods across all
drivers we currently have, to provide the same API for all drivers.

I tried to somehow make all methods consistent in this way:

# Method to query a collection of resources, returning the Array.
# The opts={} parameter might include additional filters
def instances(credentials, opts={});

# This method should return the **single** resource. The :id parameter is
# required and it must contain the resource identifier
def instance(credentials, id)

# This method might be reboot_instance or destroy_instance. All method like this
# should have the :id parameter, which must contain the resource identifier.
def instance_action(credentials, instance_id)

Note that I used 'instance' just as an example, it also apply to 'realm',
'image', etc...

So, why I'm writing this ;-) If you look to various drivers we have, you'll find
that the methods somehow defines parameters randomly.... A few examples:

def instance(credentials, opts=nil)
def instance(credentials, opts={})
def realms(credentials, opts)

etc... I found many inconsistencies like that. In order to provide at least
'usable' API, we should have the same methods with same parameters in all
drivers. The attached patch should fix them all and make the test pass:

ruby tests/api/driver_test.rb

  -- Michal


Re: [PATCH core] Core: Unified the method parameters across all drivers to have consistent drivers API

Posted by Michal Fojtik <mf...@redhat.com>.
On Aug 15, 2012, at 4:44 AM, "Koper, Dies" <di...@fast.au.fujitsu.com> wrote:

> Hi Michal,
> 
> Same from me:
> 
>>> --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
>>> @@ -822,6 +822,7 @@ end
>>> 
>> ##########################################################
>> ############
>>>   # Firewalls
>>> 
>> ##########################################################
>> ############
>>> +
>>>   def firewalls(credentials, opts={})
> 
> This newline in fgcp_driver.rb (the only line touched in this file) here
> is inconsistent with all other methods.
> I have pointed this out before, did you mean to send a different patch?

This new-line is just a mistake I overlooked. I'll correct that in the next version.
This patch need rebase, I'll send rev2 soon. Thanks for spotting this!

> 
> Regards,
> Dies Koper
> 
> 
>> -----Original Message-----
>> From: marios@redhat.com [mailto:mandreou@redhat.com]
>> Sent: Tuesday, 14 August 2012 12:24 AM
>> To: dev@deltacloud.apache.org
>> Subject: Re: [PATCH core] Core: Unified the method parameters across
> all
>> drivers to have consistent drivers API
>> 
>> nack - only because needs rebase (won't apply too old)
>> 
>> On 23/05/12 00:38, mfojtik@redhat.com wrote:
>>> From: Michal Fojtik <mf...@redhat.com>
>>> 
>>> 
>>> Signed-off-by: Michal fojtik <mf...@redhat.com>
>>> ---
>>> server/lib/deltacloud/collections/images.rb        |    2 +-
>>> server/lib/deltacloud/collections/keys.rb          |    2 +-
>>> server/lib/deltacloud/drivers/base_driver.rb       |   40
> ++++++++++--------
>> --
>>> server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |   10 ++---
>>> server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb  |    1 +
>>> .../lib/deltacloud/drivers/gogrid/gogrid_driver.rb |   12 +++---
>>> server/lib/deltacloud/drivers/mock/mock_driver.rb  |   28
> ++++++--------
>>> .../drivers/opennebula/opennebula_driver.rb        |   20
> +++++-----
>>> .../drivers/rackspace/rackspace_driver.rb          |    4 +-
>>> .../drivers/rimuhosting/rimuhosting_driver.rb      |    6 +--
>>> .../drivers/terremark/terremark_driver.rb          |    6 +--
>>> .../deltacloud/drivers/vsphere/vsphere_driver.rb   |    6 +--
>>> server/lib/deltacloud/helpers/deltacloud_helper.rb |    4 +-
>>> server/tests/api/driver_test.rb                    |   22
> +++++++----
>>> server/tests/drivers/mock/instances_test.rb        |    1 +
>>> 15 files changed, 84 insertions(+), 80 deletions(-)
>>> 
>>> diff --git a/server/lib/deltacloud/collections/images.rb
>> b/server/lib/deltacloud/collections/images.rb
>>> index 0edd7c0..85e5918 100644
>>> --- a/server/lib/deltacloud/collections/images.rb
>>> +++ b/server/lib/deltacloud/collections/images.rb
>>> @@ -58,7 +58,7 @@ module Deltacloud::Collections
>>> 
>>>       operation :destroy, :with_capability => :destroy_image do
>>>         control do
>>> -          driver.destroy_image(credentials, params[:id])
>>> +          driver.destroy_image(credentials, id)
>>>           respond_to do |format|
>>>             format.xml { status 204 }
>>>             format.json { status 204 }
>>> diff --git a/server/lib/deltacloud/collections/keys.rb
>> b/server/lib/deltacloud/collections/keys.rb
>>> index 55d0fa8..3be8e00 100644
>>> --- a/server/lib/deltacloud/collections/keys.rb
>>> +++ b/server/lib/deltacloud/collections/keys.rb
>>> @@ -45,7 +45,7 @@ module Deltacloud::Collections
>>> 
>>>       operation :destroy, :with_capability => :destroy_key do
>>>         control do
>>> -          driver.destroy_key(credentials, { :id => params[:id]})
>>> +          driver.destroy_key(credentials, params[:id])
>>>           status 204
>>>           respond_to do |format|
>>>             format.xml
>>> diff --git a/server/lib/deltacloud/drivers/base_driver.rb
>> b/server/lib/deltacloud/drivers/base_driver.rb
>>> index a360807..beb794b 100644
>>> --- a/server/lib/deltacloud/drivers/base_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/base_driver.rb
>>> @@ -115,12 +115,12 @@ module Deltacloud
>>>     def find_hardware_profile(credentials, name, image_id)
>>>       hwp = nil
>>>       if name
>>> -        unless hwp = hardware_profiles(credentials, :id =>
> name).first
>>> +        unless hwp = hardware_profile(credentials, name)
>>>           raise BackendError.new(400, "bad-hardware-profile-name",
>>>             "Hardware profile '#{name}' does not exist", nil)
>>>         end
>>>       else
>>> -        unless image = image(credentials, :id=>image_id)
>>> +        unless image = image(credentials, image_id)
>>>           raise BackendError.new(400, "bad-image-id",
>>>               "Image with ID '#{image_id}' does not exist", nil)
>>>         end
>>> @@ -206,41 +206,41 @@ module Deltacloud
>>>     # def create_firewall_rule(credentials, opts)
>>>     # def delete_firewall_rule(credentials, opts)
>>>     # def providers(credentials)
>>> -    def realm(credentials, opts)
>>> -      realms = realms(credentials, opts).first if
> has_capability?(:realms)
>>> +    def realm(credentials, id)
>>> +      realms = realms(credentials, :id => id).first if
> has_capability?(:realms)
>>>     end
>>> 
>>> -    def image(credentials, opts)
>>> -      images(credentials, opts).first if has_capability?(:images)
>>> +    def image(credentials, id)
>>> +      images(credentials, :id => id).first if
> has_capability?(:images)
>>>     end
>>> 
>>> -    def instance(credentials, opts)
>>> -      instances(credentials, opts).first if
> has_capability?(:instances)
>>> +    def instance(credentials, id)
>>> +      instances(credentials, :id => id).first if
> has_capability?(:instances)
>>>     end
>>> 
>>> -    def storage_volume(credentials, opts)
>>> -      storage_volumes(credentials, opts).first if
>> has_capability?(:storage_volumes)
>>> +    def storage_volume(credentials, id)
>>> +      storage_volumes(credentials, :id => id).first if
>> has_capability?(:storage_volumes)
>>>     end
>>> 
>>> -    def storage_snapshot(credentials, opts)
>>> -      storage_snapshots(credentials, opts).first if
>> has_capability?(:storage_snapshots)
>>> +    def storage_snapshot(credentials, id)
>>> +      storage_snapshots(credentials, :id => id).first if
>> has_capability?(:storage_snapshots)
>>>     end
>>> 
>>> -    def bucket(credentials, opts = {})
>>> +    def bucket(credentials, id)
>>>       #list of objects within bucket
>>> -      buckets(credentials, opts).first if has_capability?(:buckets)
>>> +      buckets(credentials, :id => id).first if
> has_capability?(:buckets)
>>>     end
>>> 
>>> -    def blob(credentials, opts = {})
>>> -      blobs(credentials, opts).first if has_capability?(:blobs)
>>> +    def blob(credentials, id)
>>> +      blobs(credentials, :id => id).first if
> has_capability?(:blobs)
>>>     end
>>> 
>>> -    def key(credentials, opts=nil)
>>> -      keys(credentials, opts).first if has_capability?(:keys)
>>> +    def key(credentials, id)
>>> +      keys(credentials, :id => id).first if has_capability?(:keys)
>>>     end
>>> 
>>> -    def firewall(credentials, opts={})
>>> -      firewalls(credentials, opts).first if
> has_capability?(:firewalls)
>>> +    def firewall(credentials, id)
>>> +      firewalls(credentials, :id => id).first if
> has_capability?(:firewalls)
>>>     end
>>> 
>>>     MEMBER_SHOW_METHODS =
>>> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>>> index b55132e..2fff0f9 100644
>>> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>>> @@ -183,12 +183,12 @@ module Deltacloud
>>>           end
>>>         end
>>> 
>>> -        def instance(credentials, opts={})
>>> +        def instance(credentials, id)
>>>           ec2 = new_client(credentials)
>>>           inst_arr = []
>>>           safely do
>>> -            ec2_inst = ec2.describe_instances([opts[:id]]).first
>>> -            raise "Instance #{opts[:id]} NotFound" if ec2_inst.nil?
>>> +            ec2_inst = ec2.describe_instances([id]).first
>>> +            raise "Instance #{id} NotFound" if ec2_inst.nil?
>>>             instance = convert_instance(ec2_inst)
>>>             return nil unless instance
>>>             if ec2_inst[:aws_platform] == 'windows'
>>> @@ -265,7 +265,7 @@ module Deltacloud
>>>         def reboot_instance(credentials, instance_id)
>>>           ec2 = new_client(credentials)
>>>           if ec2.reboot_instances([instance_id])
>>> -            instance(credentials, :id => instance_id)
>>> +            instance(credentials, instance_id)
>>>           else
>>>             raise Deltacloud::BackendError.new(500, "Instance",
> "Instance
>> reboot failed", "")
>>>           end
>>> @@ -274,7 +274,7 @@ module Deltacloud
>>>         def destroy_instance(credentials, instance_id)
>>>           ec2 = new_client(credentials)
>>>           if ec2.terminate_instances([instance_id])
>>> -            instance(credentials, :id => instance_id)
>>> +            instance(credentials, instance_id)
>>>           else
>>>             raise Deltacloud::BackendError.new(500, "Instance",
> "Instance
>> cannot be terminated", "")
>>>           end
>>> diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
>> b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
>>> index 4d65e7c..0edfc39 100644
>>> --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
>>> @@ -822,6 +822,7 @@ end
>>> 
>> ##########################################################
>> ############
>>>   # Firewalls
>>> 
>> ##########################################################
>> ############
>>> +
>>>   def firewalls(credentials, opts={})
>>>     firewalls = []
>>>     fw_name = 'Firewall' # currently always 'Firewall'
>>> diff --git a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
>> b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
>>> index 6469312..c87c65b 100644
>>> --- a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
>>> @@ -63,7 +63,7 @@ class GogridDriver < Deltacloud::BaseDriver
>>>     @hardware_profiles
>>>   end
>>> 
>>> -  def images(credentials, opts=nil)
>>> +  def images(credentials, opts={})
>>>     imgs = []
>>>     if opts and opts[:id]
>>>       safely do
>>> @@ -80,7 +80,7 @@ class GogridDriver < Deltacloud::BaseDriver
>>>     imgs.sort_by{|e| [e.owner_id, e.description]}
>>>   end
>>> 
>>> -  def realms(credentials, opts=nil)
>>> +  def realms(credentials, opts={})
>>>     realms = []
>>>     client = new_client(credentials)
>>>     safely do
>>> @@ -163,7 +163,7 @@ class GogridDriver < Deltacloud::BaseDriver
>>>     instances
>>>   end
>>> 
>>> -  def instances(credentials, opts=nil)
>>> +  def instances(credentials, opts={})
>>>     instances = []
>>>     if opts and opts[:id]
>>>       begin
>>> @@ -298,11 +298,11 @@ class GogridDriver < Deltacloud::BaseDriver
>>>     'Unregistering instances from load balancer is not supported in
>> GoGrid')
>>>   end
>>> 
>>> -  def key(credentials, opts=nil)
>>> -    keys(credentials, opts).first
>>> +  def key(credentials, id)
>>> +    keys(credentials, :id => id).first
>>>   end
>>> 
>>> -  def keys(credentials, opts=nil)
>>> +  def keys(credentials, opts={})
>>>     gogrid = new_client( credentials )
>>>     creds = []
>>>     safely do
>>> diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb
>> b/server/lib/deltacloud/drivers/mock/mock_driver.rb
>>> index ba817c6..6ea8764 100644
>>> --- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
>>> @@ -90,7 +90,7 @@ module Deltacloud::Drivers::Mock
>>>       @client = Client.new(storage_root)
>>>     end
>>> 
>>> -    def realms(credentials, opts=nil)
>>> +    def realms(credentials, opts={})
>>>       check_credentials( credentials )
>>>       results = []
>>>       safely do
>>> @@ -110,7 +110,7 @@ module Deltacloud::Drivers::Mock
>>>     #
>>>     # Images
>>>     #
>>> -    def images(credentials, opts=nil )
>>> +    def images(credentials, opts={} )
>>>       check_credentials( credentials )
>>>       images = []
>>>       images = @client.build_all(Image)
>>> @@ -152,14 +152,14 @@ module Deltacloud::Drivers::Mock
>>>     # Instances
>>>     #
>>> 
>>> -    def instance(credentials, opts={})
>>> +    def instance(credentials, id)
>>>       check_credentials( credentials )
>>> -      if instance = @client.load(:instances, opts[:id])
>>> +      if instance = @client.load(:instances, id)
>>>         Instance.new(instance)
>>>       end
>>>     end
>>> 
>>> -    def instances(credentials, opts=nil)
>>> +    def instances(credentials, opts={})
>>>       check_credentials( credentials )
>>>       instances = @client.build_all(Instance)
>>>       instances = filter_on( instances, :owner_id, :owner_id =>
>> credentials.user )
>>> @@ -250,14 +250,14 @@ module Deltacloud::Drivers::Mock
>>>     #
>>>     # Storage Volumes
>>>     #
>>> -    def storage_volumes(credentials, opts=nil)
>>> +    def storage_volumes(credentials, opts={})
>>>       check_credentials( credentials )
>>>       volumes = @client.build_all(StorageVolume)
>>>       volumes = filter_on( volumes, :id, opts )
>>>       volumes
>>>     end
>>> 
>>> -    def create_storage_volume(credentials, opts=nil)
>>> +    def create_storage_volume(credentials, opts={})
>>>       check_credentials(credentials)
>>>       opts ||= {}
>>>       opts[:capacity] ||= "1"
>>> @@ -273,18 +273,18 @@ module Deltacloud::Drivers::Mock
>>>       StorageVolume.new(volume)
>>>     end
>>> 
>>> -    def destroy_storage_volume(credentials, opts=nil)
>>> +    def destroy_storage_volume(credentials, id)
>>>       check_credentials(credentials)
>>> -      @client.destroy(:storage_volumes, opts[:id])
>>> +      @client.destroy(:storage_volumes, id)
>>>     end
>>> 
>>>     #opts: {:id=,:instance_id,:device}
>>> -    def attach_storage_volume(credentials, opts)
>>> +    def attach_storage_volume(credentials, opts={})
>>>       check_credentials(credentials)
>>>       attach_volume_instance(opts[:id], opts[:device],
> opts[:instance_id])
>>>     end
>>> 
>>> -    def detach_storage_volume(credentials, opts)
>>> +    def detach_storage_volume(credentials, opts={})
>>>       check_credentials(credentials)
>>>       detach_volume_instance(opts[:id], opts[:instance_id])
>>>     end
>>> @@ -293,7 +293,7 @@ module Deltacloud::Drivers::Mock
>>>     # Storage Snapshots
>>>     #
>>> 
>>> -    def storage_snapshots(credentials, opts=nil)
>>> +    def storage_snapshots(credentials, opts={})
>>>       check_credentials( credentials )
>>>       snapshots = @client.build_all(StorageSnapshot)
>>>       snapshots = filter_on(snapshots, :id, opts )
>>> @@ -307,10 +307,6 @@ module Deltacloud::Drivers::Mock
>>>       result
>>>     end
>>> 
>>> -    def key(credentials, opts={})
>>> -      keys(credentials, opts).first
>>> -    end
>>> -
>>>     def create_key(credentials, opts={})
>>>       check_credentials(credentials)
>>>       key_hash = {
>>> diff --git
>> a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
>> b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
>>> index cc1f13e..7acaf0d 100644
>>> --- a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
>>> @@ -32,7 +32,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>>> 
>> ##########################################################
>> ############
>>>   # Hardware profiles
>>> 
>> ##########################################################
>> ###########
>>> -  def hardware_profiles(credentials, opts=nil)
>>> +  def hardware_profiles(credentials, opts={})
>>>     occi_client = new_client(credentials)
>>>     xml = occi_client.get_instance_types
>>>     if CloudClient.is_error?(xml)
>>> @@ -70,8 +70,8 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>>>   ] ) unless defined?( REALMS )
>>> 
>>> 
>>> -  def realms(credentials, opts=nil)
>>> -    return REALMS if ( opts.nil? )
>>> +  def realms(credentials, opts={})
>>> +    return REALMS if ( opts.keys.empty? )
>>>     results = REALMS
>>>     results = filter_on( results, :id, opts )
>>>     results
>>> @@ -81,7 +81,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>>> 
>> ##########################################################
>> ############
>>>   # Images
>>> 
>> ##########################################################
>> ############
>>> -  def images(credentials, opts=nil)
>>> +  def images(credentials, opts={})
>>>     occi_client = new_client(credentials)
>>> 
>>>     xml = treat_response(occi_client.get_images)
>>> @@ -94,9 +94,9 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>>>     end
>>>   end
>>> 
>>> -  def image(credentials, opts=nil)
>>> +  def image(credentials, id)
>>>     occi_client = new_client(credentials)
>>> -    xml = treat_response(occi_client.get_image(opts[:id]))
>>> +    xml = treat_response(occi_client.get_image(id))
>>>     convert_image(xml, credentials)
>>>   end
>>> 
>>> @@ -153,7 +153,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>>>     stopping.to(:finish)        .automatically
>>>   end
>>> 
>>> -  def instances(credentials, opts=nil)
>>> +  def instances(credentials, opts={})
>>>     occi_client = new_client(credentials)
>>> 
>>>     xml = treat_response(occi_client.get_vms)
>>> @@ -168,13 +168,13 @@ class OpennebulaDriver <
>> Deltacloud::BaseDriver
>>>     instances = filter_on( instances, :state, opts )
>>>   end
>>> 
>>> -  def instance(credentials, opts=nil)
>>> +  def instance(credentials, id)
>>>     occi_client = new_client(credentials)
>>> -    xml = treat_response(occi_client.get_vm(opts[:id]))
>>> +    xml = treat_response(occi_client.get_vm(id))
>>>     convert_instance(xml, credentials)
>>>   end
>>> 
>>> -  def create_instance(credentials, image_id, opts=nil)
>>> +  def create_instance(credentials, image_id, opts={})
>>>     occi_client = new_client(credentials)
>>> 
>>>     storage_href = "#{occi_client.endpoint}/storage/#{image_id}"
>>> diff --git
> a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
>> b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
>>> index ca1c236..4042f3e 100644
>>> --- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
>>> @@ -46,7 +46,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
>>>     filter_hardware_profiles(results, opts)
>>>   end
>>> 
>>> -  def images(credentials, opts=nil)
>>> +  def images(credentials, opts={})
>>>     rs = new_client(credentials)
>>>     results = []
>>>     safely do
>>> @@ -65,7 +65,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
>>>   end
>>> 
>>>   #rackspace does not at this stage have realms... its all US/TX,
> all the
>> time (at least at time of writing)
>>> -  def realms(credentials, opts=nil)
>>> +  def realms(credentials, opts={})
>>>     [Realm.new( {
>>>       :id=>"us",
>>>       :name=>"United States",
>>> diff --git
>> a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
>> b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
>>> index 55eec11..bf6d561 100644
>>> ---
> a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
>>> +++
> b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
>>> @@ -26,7 +26,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
>>> 
>>>   feature :instances, :user_name
>>> 
>>> -  def images(credentails, opts=nil)
>>> +  def images(credentails, opts={})
>>>     safely do
>>>       rh = RimuHostingClient.new(credentails)
>>>       images = rh.list_images.map do | image |
>>> @@ -61,7 +61,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
>>>     filter_hardware_profiles(results, opts)
>>>   end
>>> 
>>> -  def realms(credentials, opts=nil)
>>> +  def realms(credentials, opts={})
>>>     [Realm.new( {
>>>             :id=>"rimu",
>>>             :name=>"RimuHosting",
>>> @@ -69,7 +69,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
>>>     } )]
>>>   end
>>> 
>>> -  def instances(credentials, opts=nil)
>>> +  def instances(credentials, opts={})
>>>     safely do
>>>       rh = RimuHostingClient.new(credentials)
>>>       instances = rh.list_nodes.map do | inst |
>>> diff --git
> a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
>> b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
>>> index 3260c47..ba2fb04 100644
>>> --- a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
>>> @@ -54,7 +54,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
>> "PENDING", "2" =>  "STOPPED", "4"
>>> # IMAGES
>>> #--
>>> #aka "vapp_templates"
>>> -  def images(credentials, opts=nil)
>>> +  def images(credentials, opts={})
>>>       image_list = []
>>>       terremark_client = new_client(credentials)
>>>       safely do
>>> @@ -78,7 +78,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
>> "PENDING", "2" =>  "STOPPED", "4"
>>> # REALMS
>>> #--
>>> #only one realm... everything in US?
>>> -  def realms(credentials, opts=nil)
>>> +  def realms(credentials, opts={})
>>>      [Realm.new( {
>>>       :id=>"US-Miami",
>>>       :name=>"United States - Miami",
>>> @@ -90,7 +90,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
>> "PENDING", "2" =>  "STOPPED", "4"
>>> # INSTANCES
>>> #--
>>> #aka vApps
>>> -  def instances(credentials, opts=nil)
>>> +  def instances(credentials, opts={})
>>>       instances = []
>>>       terremark_client = new_client(credentials)
>>>       safely do
>>> diff --git a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
>> b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
>>> index 253a2e5..7b1b1e4 100644
>>> --- a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
>>> +++ b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
>>> @@ -73,7 +73,7 @@ module Deltacloud::Drivers::Vsphere
>>> 
>>>     # Images are virtual machines with 'template' flag set to be
> true.
>>>     # Thus we're getting them using find_vm and
> list_virtual_machines
>>> -    def images(credentials, opts=nil)
>>> +    def images(credentials, opts={})
>>>       cloud = new_client(credentials)
>>>       img_arr = []
>>>       profiles = hardware_profiles(credentials)
>>> @@ -123,7 +123,7 @@ module Deltacloud::Drivers::Vsphere
>>>     end
>>> 
>>>     # List all datacenters managed by the vSphere or vCenter
> entrypoint.
>>> -    def realms(credentials, opts=nil)
>>> +    def realms(credentials, opts={})
>>>       vsphere = new_client(credentials)
>>>       safely do
>>>         if opts and opts[:id]
>>> @@ -140,7 +140,7 @@ module Deltacloud::Drivers::Vsphere
>>> 
>>>     # List all running instances, across all datacenters.
> DeltaCloud API does
>>>     # not yet support filtering instances by realm.
>>> -    def instances(credentials, opts=nil)
>>> +    def instances(credentials, opts={})
>>>       cloud = new_client(credentials)
>>>       inst_arr, machine_vms, pending_vms = [], [], []
>>>       safely do
>>> diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb
>> b/server/lib/deltacloud/helpers/deltacloud_helper.rb
>>> index 4ad0be9..ef107c7 100644
>>> --- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
>>> +++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
>>> @@ -79,7 +79,7 @@ module Deltacloud::Helpers
>>> 
>>>     def show(model)
>>>       @benchmark = Benchmark.measure do
>>> -        @element = driver.send(model, credentials, { :id =>
> params[:id]} )
>>> +        @element = driver.send(model, credentials, params[:id])
>>>       end
>>>       headers['X-Backend-Runtime'] = @benchmark.real.to_s
>>>       instance_variable_set("@#{model}", @element)
>>> @@ -105,7 +105,7 @@ module Deltacloud::Helpers
>>>     end
>>> 
>>>     def instance_action(name)
>>> -      original_instance = driver.instance(credentials, :id =>
> params[:id])
>>> +      original_instance = driver.instance(credentials, params[:id])
>>> 
>>>       # If original instance doesn't include called action
>>>       # return with 405 error (Method is not Allowed)
>>> diff --git a/server/tests/api/driver_test.rb
>> b/server/tests/api/driver_test.rb
>>> index c9facbf..2ec22ef 100644
>>> --- a/server/tests/api/driver_test.rb
>>> +++ b/server/tests/api/driver_test.rb
>>> @@ -47,19 +47,19 @@ describe 'Deltacloud drivers API' do
>>> 
>>>   METHODS = {
>>>     :firewalls => [[:credentials], [:opts, "{  }"]],
>>> -    :firewall  => [[:credentials], [:opts, "{  }"]],
>>> +    :firewall  => [[:credentials], [:id,]],
>>>     :keys    => [[:credentials], [:opts, "{  }"]],
>>> -    :key     => [[:credentials], [:opts]],
>>> +    :key     => [[:credentials], [:id]],
>>>     :storage_snapshots => [[:credentials], [:opts, "{  }"]],
>>> -    :storage_snapshot  => [[:credentials], [:opts]],
>>> +    :storage_snapshot  => [[:credentials], [:id]],
>>>     :storage_volumes => [[:credentials], [:opts, "{  }"]],
>>> -    :storage_volume  => [[:credentials], [:opts]],
>>> +    :storage_volume  => [[:credentials], [:id]],
>>>     :realms    => [[:credentials], [:opts, "{  }"]],
>>> -    :realm     => [[:credentials], [:opts]],
>>> +    :realm     => [[:credentials], [:id]],
>>>     :images    => [[:credentials], [:opts, "{  }"]],
>>> -    :image     => [[:credentials], [:opts]],
>>> +    :image     => [[:credentials], [:id]],
>>>     :instances => [[:credentials], [:opts, "{  }"]],
>>> -    :instance  => [[:credentials], [:opts]],
>>> +    :instance  => [[:credentials], [:id]],
>>>     :create_instance => [[:credentials], [:image_id], [:opts, "{
> }"]],
>>>     :destroy_instance => [[:credentials], [:id]],
>>>     :stop_instance => [[:credentials], [:id]],
>>> @@ -71,7 +71,13 @@ describe 'Deltacloud drivers API' do
>>>     METHODS.each do |m, definition|
>>>       it "should have the correct parameters for the :#{m} method
> in #{key}
>> driver" do
>>>         next unless Deltacloud.new(key).backend.respond_to? m
>>> -        Arguments.names(Deltacloud.new(key).backend.class,
>> m).must_equal definition
>>> +        is_optional = (definition[1][1] and definition[1][1] == '{
> }')
>>> +        if is_optional
>>> +          Arguments.names(Deltacloud.new(key).backend.class,
>> m)[1][1].wont_be_nil
>>> +          Arguments.names(Deltacloud.new(key).backend.class,
>> m)[1][1].must_equal '{  }'
>>> +        else
>>> +          Arguments.names(Deltacloud.new(key).backend.class,
>> m)[1][1].must_be_nil
>>> +        end
>>>       end
>>>     end
>>>   end
>>> diff --git a/server/tests/drivers/mock/instances_test.rb
>> b/server/tests/drivers/mock/instances_test.rb
>>> index d44fac5..0855190 100644
>>> --- a/server/tests/drivers/mock/instances_test.rb
>>> +++ b/server/tests/drivers/mock/instances_test.rb
>>> @@ -261,6 +261,7 @@ describe 'Deltacloud API instances' do
>>>       :image_id => image_id,
>>>       :realm_id => realm_id,
>>>     }
>>> +    puts last_response.body
>>>     last_response.status.must_equal 201 # HTTP_CREATED
>>>     last_response.headers['Location'].wont_be_nil # Location header
> must
>> be set, pointing to new the instance
>>>     instance_id = last_response.headers['Location'].split('/').last
>>> 
>> 
>> 
> 
> 


RE: [PATCH core] Core: Unified the method parameters across all drivers to have consistent drivers API

Posted by "Koper, Dies" <di...@fast.au.fujitsu.com>.
Hi Michal,

Same from me:

> > --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> > +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> > @@ -822,6 +822,7 @@ end
> >
> ##########################################################
> ############
> >    # Firewalls
> >
> ##########################################################
> ############
> > +
> >    def firewalls(credentials, opts={})

This newline in fgcp_driver.rb (the only line touched in this file) here
is inconsistent with all other methods.
I have pointed this out before, did you mean to send a different patch?

Regards,
Dies Koper


> -----Original Message-----
> From: marios@redhat.com [mailto:mandreou@redhat.com]
> Sent: Tuesday, 14 August 2012 12:24 AM
> To: dev@deltacloud.apache.org
> Subject: Re: [PATCH core] Core: Unified the method parameters across
all
> drivers to have consistent drivers API
> 
> nack - only because needs rebase (won't apply too old)
> 
> On 23/05/12 00:38, mfojtik@redhat.com wrote:
> > From: Michal Fojtik <mf...@redhat.com>
> >
> >
> > Signed-off-by: Michal fojtik <mf...@redhat.com>
> > ---
> >  server/lib/deltacloud/collections/images.rb        |    2 +-
> >  server/lib/deltacloud/collections/keys.rb          |    2 +-
> >  server/lib/deltacloud/drivers/base_driver.rb       |   40
++++++++++--------
> --
> >  server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |   10 ++---
> >  server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb  |    1 +
> >  .../lib/deltacloud/drivers/gogrid/gogrid_driver.rb |   12 +++---
> >  server/lib/deltacloud/drivers/mock/mock_driver.rb  |   28
++++++--------
> >  .../drivers/opennebula/opennebula_driver.rb        |   20
+++++-----
> >  .../drivers/rackspace/rackspace_driver.rb          |    4 +-
> >  .../drivers/rimuhosting/rimuhosting_driver.rb      |    6 +--
> >  .../drivers/terremark/terremark_driver.rb          |    6 +--
> >  .../deltacloud/drivers/vsphere/vsphere_driver.rb   |    6 +--
> >  server/lib/deltacloud/helpers/deltacloud_helper.rb |    4 +-
> >  server/tests/api/driver_test.rb                    |   22
+++++++----
> >  server/tests/drivers/mock/instances_test.rb        |    1 +
> >  15 files changed, 84 insertions(+), 80 deletions(-)
> >
> > diff --git a/server/lib/deltacloud/collections/images.rb
> b/server/lib/deltacloud/collections/images.rb
> > index 0edd7c0..85e5918 100644
> > --- a/server/lib/deltacloud/collections/images.rb
> > +++ b/server/lib/deltacloud/collections/images.rb
> > @@ -58,7 +58,7 @@ module Deltacloud::Collections
> >
> >        operation :destroy, :with_capability => :destroy_image do
> >          control do
> > -          driver.destroy_image(credentials, params[:id])
> > +          driver.destroy_image(credentials, id)
> >            respond_to do |format|
> >              format.xml { status 204 }
> >              format.json { status 204 }
> > diff --git a/server/lib/deltacloud/collections/keys.rb
> b/server/lib/deltacloud/collections/keys.rb
> > index 55d0fa8..3be8e00 100644
> > --- a/server/lib/deltacloud/collections/keys.rb
> > +++ b/server/lib/deltacloud/collections/keys.rb
> > @@ -45,7 +45,7 @@ module Deltacloud::Collections
> >
> >        operation :destroy, :with_capability => :destroy_key do
> >          control do
> > -          driver.destroy_key(credentials, { :id => params[:id]})
> > +          driver.destroy_key(credentials, params[:id])
> >            status 204
> >            respond_to do |format|
> >              format.xml
> > diff --git a/server/lib/deltacloud/drivers/base_driver.rb
> b/server/lib/deltacloud/drivers/base_driver.rb
> > index a360807..beb794b 100644
> > --- a/server/lib/deltacloud/drivers/base_driver.rb
> > +++ b/server/lib/deltacloud/drivers/base_driver.rb
> > @@ -115,12 +115,12 @@ module Deltacloud
> >      def find_hardware_profile(credentials, name, image_id)
> >        hwp = nil
> >        if name
> > -        unless hwp = hardware_profiles(credentials, :id =>
name).first
> > +        unless hwp = hardware_profile(credentials, name)
> >            raise BackendError.new(400, "bad-hardware-profile-name",
> >              "Hardware profile '#{name}' does not exist", nil)
> >          end
> >        else
> > -        unless image = image(credentials, :id=>image_id)
> > +        unless image = image(credentials, image_id)
> >            raise BackendError.new(400, "bad-image-id",
> >                "Image with ID '#{image_id}' does not exist", nil)
> >          end
> > @@ -206,41 +206,41 @@ module Deltacloud
> >      # def create_firewall_rule(credentials, opts)
> >      # def delete_firewall_rule(credentials, opts)
> >      # def providers(credentials)
> > -    def realm(credentials, opts)
> > -      realms = realms(credentials, opts).first if
has_capability?(:realms)
> > +    def realm(credentials, id)
> > +      realms = realms(credentials, :id => id).first if
has_capability?(:realms)
> >      end
> >
> > -    def image(credentials, opts)
> > -      images(credentials, opts).first if has_capability?(:images)
> > +    def image(credentials, id)
> > +      images(credentials, :id => id).first if
has_capability?(:images)
> >      end
> >
> > -    def instance(credentials, opts)
> > -      instances(credentials, opts).first if
has_capability?(:instances)
> > +    def instance(credentials, id)
> > +      instances(credentials, :id => id).first if
has_capability?(:instances)
> >      end
> >
> > -    def storage_volume(credentials, opts)
> > -      storage_volumes(credentials, opts).first if
> has_capability?(:storage_volumes)
> > +    def storage_volume(credentials, id)
> > +      storage_volumes(credentials, :id => id).first if
> has_capability?(:storage_volumes)
> >      end
> >
> > -    def storage_snapshot(credentials, opts)
> > -      storage_snapshots(credentials, opts).first if
> has_capability?(:storage_snapshots)
> > +    def storage_snapshot(credentials, id)
> > +      storage_snapshots(credentials, :id => id).first if
> has_capability?(:storage_snapshots)
> >      end
> >
> > -    def bucket(credentials, opts = {})
> > +    def bucket(credentials, id)
> >        #list of objects within bucket
> > -      buckets(credentials, opts).first if has_capability?(:buckets)
> > +      buckets(credentials, :id => id).first if
has_capability?(:buckets)
> >      end
> >
> > -    def blob(credentials, opts = {})
> > -      blobs(credentials, opts).first if has_capability?(:blobs)
> > +    def blob(credentials, id)
> > +      blobs(credentials, :id => id).first if
has_capability?(:blobs)
> >      end
> >
> > -    def key(credentials, opts=nil)
> > -      keys(credentials, opts).first if has_capability?(:keys)
> > +    def key(credentials, id)
> > +      keys(credentials, :id => id).first if has_capability?(:keys)
> >      end
> >
> > -    def firewall(credentials, opts={})
> > -      firewalls(credentials, opts).first if
has_capability?(:firewalls)
> > +    def firewall(credentials, id)
> > +      firewalls(credentials, :id => id).first if
has_capability?(:firewalls)
> >      end
> >
> >      MEMBER_SHOW_METHODS =
> > diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> > index b55132e..2fff0f9 100644
> > --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> > +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> > @@ -183,12 +183,12 @@ module Deltacloud
> >            end
> >          end
> >
> > -        def instance(credentials, opts={})
> > +        def instance(credentials, id)
> >            ec2 = new_client(credentials)
> >            inst_arr = []
> >            safely do
> > -            ec2_inst = ec2.describe_instances([opts[:id]]).first
> > -            raise "Instance #{opts[:id]} NotFound" if ec2_inst.nil?
> > +            ec2_inst = ec2.describe_instances([id]).first
> > +            raise "Instance #{id} NotFound" if ec2_inst.nil?
> >              instance = convert_instance(ec2_inst)
> >              return nil unless instance
> >              if ec2_inst[:aws_platform] == 'windows'
> > @@ -265,7 +265,7 @@ module Deltacloud
> >          def reboot_instance(credentials, instance_id)
> >            ec2 = new_client(credentials)
> >            if ec2.reboot_instances([instance_id])
> > -            instance(credentials, :id => instance_id)
> > +            instance(credentials, instance_id)
> >            else
> >              raise Deltacloud::BackendError.new(500, "Instance",
"Instance
> reboot failed", "")
> >            end
> > @@ -274,7 +274,7 @@ module Deltacloud
> >          def destroy_instance(credentials, instance_id)
> >            ec2 = new_client(credentials)
> >            if ec2.terminate_instances([instance_id])
> > -            instance(credentials, :id => instance_id)
> > +            instance(credentials, instance_id)
> >            else
> >              raise Deltacloud::BackendError.new(500, "Instance",
"Instance
> cannot be terminated", "")
> >            end
> > diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> > index 4d65e7c..0edfc39 100644
> > --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> > +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> > @@ -822,6 +822,7 @@ end
> >
> ##########################################################
> ############
> >    # Firewalls
> >
> ##########################################################
> ############
> > +
> >    def firewalls(credentials, opts={})
> >      firewalls = []
> >      fw_name = 'Firewall' # currently always 'Firewall'
> > diff --git a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> > index 6469312..c87c65b 100644
> > --- a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> > +++ b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> > @@ -63,7 +63,7 @@ class GogridDriver < Deltacloud::BaseDriver
> >      @hardware_profiles
> >    end
> >
> > -  def images(credentials, opts=nil)
> > +  def images(credentials, opts={})
> >      imgs = []
> >      if opts and opts[:id]
> >        safely do
> > @@ -80,7 +80,7 @@ class GogridDriver < Deltacloud::BaseDriver
> >      imgs.sort_by{|e| [e.owner_id, e.description]}
> >    end
> >
> > -  def realms(credentials, opts=nil)
> > +  def realms(credentials, opts={})
> >      realms = []
> >      client = new_client(credentials)
> >      safely do
> > @@ -163,7 +163,7 @@ class GogridDriver < Deltacloud::BaseDriver
> >      instances
> >    end
> >
> > -  def instances(credentials, opts=nil)
> > +  def instances(credentials, opts={})
> >      instances = []
> >      if opts and opts[:id]
> >        begin
> > @@ -298,11 +298,11 @@ class GogridDriver < Deltacloud::BaseDriver
> >      'Unregistering instances from load balancer is not supported in
> GoGrid')
> >    end
> >
> > -  def key(credentials, opts=nil)
> > -    keys(credentials, opts).first
> > +  def key(credentials, id)
> > +    keys(credentials, :id => id).first
> >    end
> >
> > -  def keys(credentials, opts=nil)
> > +  def keys(credentials, opts={})
> >      gogrid = new_client( credentials )
> >      creds = []
> >      safely do
> > diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb
> b/server/lib/deltacloud/drivers/mock/mock_driver.rb
> > index ba817c6..6ea8764 100644
> > --- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
> > +++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
> > @@ -90,7 +90,7 @@ module Deltacloud::Drivers::Mock
> >        @client = Client.new(storage_root)
> >      end
> >
> > -    def realms(credentials, opts=nil)
> > +    def realms(credentials, opts={})
> >        check_credentials( credentials )
> >        results = []
> >        safely do
> > @@ -110,7 +110,7 @@ module Deltacloud::Drivers::Mock
> >      #
> >      # Images
> >      #
> > -    def images(credentials, opts=nil )
> > +    def images(credentials, opts={} )
> >        check_credentials( credentials )
> >        images = []
> >        images = @client.build_all(Image)
> > @@ -152,14 +152,14 @@ module Deltacloud::Drivers::Mock
> >      # Instances
> >      #
> >
> > -    def instance(credentials, opts={})
> > +    def instance(credentials, id)
> >        check_credentials( credentials )
> > -      if instance = @client.load(:instances, opts[:id])
> > +      if instance = @client.load(:instances, id)
> >          Instance.new(instance)
> >        end
> >      end
> >
> > -    def instances(credentials, opts=nil)
> > +    def instances(credentials, opts={})
> >        check_credentials( credentials )
> >        instances = @client.build_all(Instance)
> >        instances = filter_on( instances, :owner_id, :owner_id =>
> credentials.user )
> > @@ -250,14 +250,14 @@ module Deltacloud::Drivers::Mock
> >      #
> >      # Storage Volumes
> >      #
> > -    def storage_volumes(credentials, opts=nil)
> > +    def storage_volumes(credentials, opts={})
> >        check_credentials( credentials )
> >        volumes = @client.build_all(StorageVolume)
> >        volumes = filter_on( volumes, :id, opts )
> >        volumes
> >      end
> >
> > -    def create_storage_volume(credentials, opts=nil)
> > +    def create_storage_volume(credentials, opts={})
> >        check_credentials(credentials)
> >        opts ||= {}
> >        opts[:capacity] ||= "1"
> > @@ -273,18 +273,18 @@ module Deltacloud::Drivers::Mock
> >        StorageVolume.new(volume)
> >      end
> >
> > -    def destroy_storage_volume(credentials, opts=nil)
> > +    def destroy_storage_volume(credentials, id)
> >        check_credentials(credentials)
> > -      @client.destroy(:storage_volumes, opts[:id])
> > +      @client.destroy(:storage_volumes, id)
> >      end
> >
> >      #opts: {:id=,:instance_id,:device}
> > -    def attach_storage_volume(credentials, opts)
> > +    def attach_storage_volume(credentials, opts={})
> >        check_credentials(credentials)
> >        attach_volume_instance(opts[:id], opts[:device],
opts[:instance_id])
> >      end
> >
> > -    def detach_storage_volume(credentials, opts)
> > +    def detach_storage_volume(credentials, opts={})
> >        check_credentials(credentials)
> >        detach_volume_instance(opts[:id], opts[:instance_id])
> >      end
> > @@ -293,7 +293,7 @@ module Deltacloud::Drivers::Mock
> >      # Storage Snapshots
> >      #
> >
> > -    def storage_snapshots(credentials, opts=nil)
> > +    def storage_snapshots(credentials, opts={})
> >        check_credentials( credentials )
> >        snapshots = @client.build_all(StorageSnapshot)
> >        snapshots = filter_on(snapshots, :id, opts )
> > @@ -307,10 +307,6 @@ module Deltacloud::Drivers::Mock
> >        result
> >      end
> >
> > -    def key(credentials, opts={})
> > -      keys(credentials, opts).first
> > -    end
> > -
> >      def create_key(credentials, opts={})
> >        check_credentials(credentials)
> >        key_hash = {
> > diff --git
> a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> > index cc1f13e..7acaf0d 100644
> > --- a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> > +++ b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> > @@ -32,7 +32,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
> >
> ##########################################################
> ############
> >    # Hardware profiles
> >
> ##########################################################
> ###########
> > -  def hardware_profiles(credentials, opts=nil)
> > +  def hardware_profiles(credentials, opts={})
> >      occi_client = new_client(credentials)
> >      xml = occi_client.get_instance_types
> >      if CloudClient.is_error?(xml)
> > @@ -70,8 +70,8 @@ class OpennebulaDriver < Deltacloud::BaseDriver
> >    ] ) unless defined?( REALMS )
> >
> >
> > -  def realms(credentials, opts=nil)
> > -    return REALMS if ( opts.nil? )
> > +  def realms(credentials, opts={})
> > +    return REALMS if ( opts.keys.empty? )
> >      results = REALMS
> >      results = filter_on( results, :id, opts )
> >      results
> > @@ -81,7 +81,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
> >
> ##########################################################
> ############
> >    # Images
> >
> ##########################################################
> ############
> > -  def images(credentials, opts=nil)
> > +  def images(credentials, opts={})
> >      occi_client = new_client(credentials)
> >
> >      xml = treat_response(occi_client.get_images)
> > @@ -94,9 +94,9 @@ class OpennebulaDriver < Deltacloud::BaseDriver
> >      end
> >    end
> >
> > -  def image(credentials, opts=nil)
> > +  def image(credentials, id)
> >      occi_client = new_client(credentials)
> > -    xml = treat_response(occi_client.get_image(opts[:id]))
> > +    xml = treat_response(occi_client.get_image(id))
> >      convert_image(xml, credentials)
> >    end
> >
> > @@ -153,7 +153,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
> >      stopping.to(:finish)        .automatically
> >    end
> >
> > -  def instances(credentials, opts=nil)
> > +  def instances(credentials, opts={})
> >      occi_client = new_client(credentials)
> >
> >      xml = treat_response(occi_client.get_vms)
> > @@ -168,13 +168,13 @@ class OpennebulaDriver <
> Deltacloud::BaseDriver
> >      instances = filter_on( instances, :state, opts )
> >    end
> >
> > -  def instance(credentials, opts=nil)
> > +  def instance(credentials, id)
> >      occi_client = new_client(credentials)
> > -    xml = treat_response(occi_client.get_vm(opts[:id]))
> > +    xml = treat_response(occi_client.get_vm(id))
> >      convert_instance(xml, credentials)
> >    end
> >
> > -  def create_instance(credentials, image_id, opts=nil)
> > +  def create_instance(credentials, image_id, opts={})
> >      occi_client = new_client(credentials)
> >
> >      storage_href = "#{occi_client.endpoint}/storage/#{image_id}"
> > diff --git
a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> > index ca1c236..4042f3e 100644
> > --- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> > +++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> > @@ -46,7 +46,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
> >      filter_hardware_profiles(results, opts)
> >    end
> >
> > -  def images(credentials, opts=nil)
> > +  def images(credentials, opts={})
> >      rs = new_client(credentials)
> >      results = []
> >      safely do
> > @@ -65,7 +65,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
> >    end
> >
> >    #rackspace does not at this stage have realms... its all US/TX,
all the
> time (at least at time of writing)
> > -  def realms(credentials, opts=nil)
> > +  def realms(credentials, opts={})
> >      [Realm.new( {
> >        :id=>"us",
> >        :name=>"United States",
> > diff --git
> a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> > index 55eec11..bf6d561 100644
> > ---
a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> > +++
b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> > @@ -26,7 +26,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
> >
> >    feature :instances, :user_name
> >
> > -  def images(credentails, opts=nil)
> > +  def images(credentails, opts={})
> >      safely do
> >        rh = RimuHostingClient.new(credentails)
> >        images = rh.list_images.map do | image |
> > @@ -61,7 +61,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
> >      filter_hardware_profiles(results, opts)
> >    end
> >
> > -  def realms(credentials, opts=nil)
> > +  def realms(credentials, opts={})
> >      [Realm.new( {
> >              :id=>"rimu",
> >              :name=>"RimuHosting",
> > @@ -69,7 +69,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
> >      } )]
> >    end
> >
> > -  def instances(credentials, opts=nil)
> > +  def instances(credentials, opts={})
> >      safely do
> >        rh = RimuHostingClient.new(credentials)
> >        instances = rh.list_nodes.map do | inst |
> > diff --git
a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> > index 3260c47..ba2fb04 100644
> > --- a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> > +++ b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> > @@ -54,7 +54,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
> "PENDING", "2" =>  "STOPPED", "4"
> >  # IMAGES
> >  #--
> >  #aka "vapp_templates"
> > -  def images(credentials, opts=nil)
> > +  def images(credentials, opts={})
> >        image_list = []
> >        terremark_client = new_client(credentials)
> >        safely do
> > @@ -78,7 +78,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
> "PENDING", "2" =>  "STOPPED", "4"
> >  # REALMS
> >  #--
> >  #only one realm... everything in US?
> > -  def realms(credentials, opts=nil)
> > +  def realms(credentials, opts={})
> >       [Realm.new( {
> >        :id=>"US-Miami",
> >        :name=>"United States - Miami",
> > @@ -90,7 +90,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
> "PENDING", "2" =>  "STOPPED", "4"
> >  # INSTANCES
> >  #--
> >  #aka vApps
> > -  def instances(credentials, opts=nil)
> > +  def instances(credentials, opts={})
> >        instances = []
> >        terremark_client = new_client(credentials)
> >        safely do
> > diff --git a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> > index 253a2e5..7b1b1e4 100644
> > --- a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> > +++ b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> > @@ -73,7 +73,7 @@ module Deltacloud::Drivers::Vsphere
> >
> >      # Images are virtual machines with 'template' flag set to be
true.
> >      # Thus we're getting them using find_vm and
list_virtual_machines
> > -    def images(credentials, opts=nil)
> > +    def images(credentials, opts={})
> >        cloud = new_client(credentials)
> >        img_arr = []
> >        profiles = hardware_profiles(credentials)
> > @@ -123,7 +123,7 @@ module Deltacloud::Drivers::Vsphere
> >      end
> >
> >      # List all datacenters managed by the vSphere or vCenter
entrypoint.
> > -    def realms(credentials, opts=nil)
> > +    def realms(credentials, opts={})
> >        vsphere = new_client(credentials)
> >        safely do
> >          if opts and opts[:id]
> > @@ -140,7 +140,7 @@ module Deltacloud::Drivers::Vsphere
> >
> >      # List all running instances, across all datacenters.
DeltaCloud API does
> >      # not yet support filtering instances by realm.
> > -    def instances(credentials, opts=nil)
> > +    def instances(credentials, opts={})
> >        cloud = new_client(credentials)
> >        inst_arr, machine_vms, pending_vms = [], [], []
> >        safely do
> > diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb
> b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> > index 4ad0be9..ef107c7 100644
> > --- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
> > +++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> > @@ -79,7 +79,7 @@ module Deltacloud::Helpers
> >
> >      def show(model)
> >        @benchmark = Benchmark.measure do
> > -        @element = driver.send(model, credentials, { :id =>
params[:id]} )
> > +        @element = driver.send(model, credentials, params[:id])
> >        end
> >        headers['X-Backend-Runtime'] = @benchmark.real.to_s
> >        instance_variable_set("@#{model}", @element)
> > @@ -105,7 +105,7 @@ module Deltacloud::Helpers
> >      end
> >
> >      def instance_action(name)
> > -      original_instance = driver.instance(credentials, :id =>
params[:id])
> > +      original_instance = driver.instance(credentials, params[:id])
> >
> >        # If original instance doesn't include called action
> >        # return with 405 error (Method is not Allowed)
> > diff --git a/server/tests/api/driver_test.rb
> b/server/tests/api/driver_test.rb
> > index c9facbf..2ec22ef 100644
> > --- a/server/tests/api/driver_test.rb
> > +++ b/server/tests/api/driver_test.rb
> > @@ -47,19 +47,19 @@ describe 'Deltacloud drivers API' do
> >
> >    METHODS = {
> >      :firewalls => [[:credentials], [:opts, "{  }"]],
> > -    :firewall  => [[:credentials], [:opts, "{  }"]],
> > +    :firewall  => [[:credentials], [:id,]],
> >      :keys    => [[:credentials], [:opts, "{  }"]],
> > -    :key     => [[:credentials], [:opts]],
> > +    :key     => [[:credentials], [:id]],
> >      :storage_snapshots => [[:credentials], [:opts, "{  }"]],
> > -    :storage_snapshot  => [[:credentials], [:opts]],
> > +    :storage_snapshot  => [[:credentials], [:id]],
> >      :storage_volumes => [[:credentials], [:opts, "{  }"]],
> > -    :storage_volume  => [[:credentials], [:opts]],
> > +    :storage_volume  => [[:credentials], [:id]],
> >      :realms    => [[:credentials], [:opts, "{  }"]],
> > -    :realm     => [[:credentials], [:opts]],
> > +    :realm     => [[:credentials], [:id]],
> >      :images    => [[:credentials], [:opts, "{  }"]],
> > -    :image     => [[:credentials], [:opts]],
> > +    :image     => [[:credentials], [:id]],
> >      :instances => [[:credentials], [:opts, "{  }"]],
> > -    :instance  => [[:credentials], [:opts]],
> > +    :instance  => [[:credentials], [:id]],
> >      :create_instance => [[:credentials], [:image_id], [:opts, "{
}"]],
> >      :destroy_instance => [[:credentials], [:id]],
> >      :stop_instance => [[:credentials], [:id]],
> > @@ -71,7 +71,13 @@ describe 'Deltacloud drivers API' do
> >      METHODS.each do |m, definition|
> >        it "should have the correct parameters for the :#{m} method
in #{key}
> driver" do
> >          next unless Deltacloud.new(key).backend.respond_to? m
> > -        Arguments.names(Deltacloud.new(key).backend.class,
> m).must_equal definition
> > +        is_optional = (definition[1][1] and definition[1][1] == '{
}')
> > +        if is_optional
> > +          Arguments.names(Deltacloud.new(key).backend.class,
> m)[1][1].wont_be_nil
> > +          Arguments.names(Deltacloud.new(key).backend.class,
> m)[1][1].must_equal '{  }'
> > +        else
> > +          Arguments.names(Deltacloud.new(key).backend.class,
> m)[1][1].must_be_nil
> > +        end
> >        end
> >      end
> >    end
> > diff --git a/server/tests/drivers/mock/instances_test.rb
> b/server/tests/drivers/mock/instances_test.rb
> > index d44fac5..0855190 100644
> > --- a/server/tests/drivers/mock/instances_test.rb
> > +++ b/server/tests/drivers/mock/instances_test.rb
> > @@ -261,6 +261,7 @@ describe 'Deltacloud API instances' do
> >        :image_id => image_id,
> >        :realm_id => realm_id,
> >      }
> > +    puts last_response.body
> >      last_response.status.must_equal 201 # HTTP_CREATED
> >      last_response.headers['Location'].wont_be_nil # Location header
must
> be set, pointing to new the instance
> >      instance_id = last_response.headers['Location'].split('/').last
> >
> 
> 



Re: [PATCH core] Core: Unified the method parameters across all drivers to have consistent drivers API

Posted by "marios@redhat.com" <ma...@redhat.com>.
nack - only because needs rebase (won't apply too old)

On 23/05/12 00:38, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
>  server/lib/deltacloud/collections/images.rb        |    2 +-
>  server/lib/deltacloud/collections/keys.rb          |    2 +-
>  server/lib/deltacloud/drivers/base_driver.rb       |   40 ++++++++++----------
>  server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |   10 ++---
>  server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb  |    1 +
>  .../lib/deltacloud/drivers/gogrid/gogrid_driver.rb |   12 +++---
>  server/lib/deltacloud/drivers/mock/mock_driver.rb  |   28 ++++++--------
>  .../drivers/opennebula/opennebula_driver.rb        |   20 +++++-----
>  .../drivers/rackspace/rackspace_driver.rb          |    4 +-
>  .../drivers/rimuhosting/rimuhosting_driver.rb      |    6 +--
>  .../drivers/terremark/terremark_driver.rb          |    6 +--
>  .../deltacloud/drivers/vsphere/vsphere_driver.rb   |    6 +--
>  server/lib/deltacloud/helpers/deltacloud_helper.rb |    4 +-
>  server/tests/api/driver_test.rb                    |   22 +++++++----
>  server/tests/drivers/mock/instances_test.rb        |    1 +
>  15 files changed, 84 insertions(+), 80 deletions(-)
> 
> diff --git a/server/lib/deltacloud/collections/images.rb b/server/lib/deltacloud/collections/images.rb
> index 0edd7c0..85e5918 100644
> --- a/server/lib/deltacloud/collections/images.rb
> +++ b/server/lib/deltacloud/collections/images.rb
> @@ -58,7 +58,7 @@ module Deltacloud::Collections
>  
>        operation :destroy, :with_capability => :destroy_image do
>          control do
> -          driver.destroy_image(credentials, params[:id])
> +          driver.destroy_image(credentials, id)
>            respond_to do |format|
>              format.xml { status 204 }
>              format.json { status 204 }
> diff --git a/server/lib/deltacloud/collections/keys.rb b/server/lib/deltacloud/collections/keys.rb
> index 55d0fa8..3be8e00 100644
> --- a/server/lib/deltacloud/collections/keys.rb
> +++ b/server/lib/deltacloud/collections/keys.rb
> @@ -45,7 +45,7 @@ module Deltacloud::Collections
>  
>        operation :destroy, :with_capability => :destroy_key do
>          control do
> -          driver.destroy_key(credentials, { :id => params[:id]})
> +          driver.destroy_key(credentials, params[:id])
>            status 204
>            respond_to do |format|
>              format.xml
> diff --git a/server/lib/deltacloud/drivers/base_driver.rb b/server/lib/deltacloud/drivers/base_driver.rb
> index a360807..beb794b 100644
> --- a/server/lib/deltacloud/drivers/base_driver.rb
> +++ b/server/lib/deltacloud/drivers/base_driver.rb
> @@ -115,12 +115,12 @@ module Deltacloud
>      def find_hardware_profile(credentials, name, image_id)
>        hwp = nil
>        if name
> -        unless hwp = hardware_profiles(credentials, :id => name).first
> +        unless hwp = hardware_profile(credentials, name)
>            raise BackendError.new(400, "bad-hardware-profile-name",
>              "Hardware profile '#{name}' does not exist", nil)
>          end
>        else
> -        unless image = image(credentials, :id=>image_id)
> +        unless image = image(credentials, image_id)
>            raise BackendError.new(400, "bad-image-id",
>                "Image with ID '#{image_id}' does not exist", nil)
>          end
> @@ -206,41 +206,41 @@ module Deltacloud
>      # def create_firewall_rule(credentials, opts)
>      # def delete_firewall_rule(credentials, opts)
>      # def providers(credentials)
> -    def realm(credentials, opts)
> -      realms = realms(credentials, opts).first if has_capability?(:realms)
> +    def realm(credentials, id)
> +      realms = realms(credentials, :id => id).first if has_capability?(:realms)
>      end
>  
> -    def image(credentials, opts)
> -      images(credentials, opts).first if has_capability?(:images)
> +    def image(credentials, id)
> +      images(credentials, :id => id).first if has_capability?(:images)
>      end
>  
> -    def instance(credentials, opts)
> -      instances(credentials, opts).first if has_capability?(:instances)
> +    def instance(credentials, id)
> +      instances(credentials, :id => id).first if has_capability?(:instances)
>      end
>  
> -    def storage_volume(credentials, opts)
> -      storage_volumes(credentials, opts).first if has_capability?(:storage_volumes)
> +    def storage_volume(credentials, id)
> +      storage_volumes(credentials, :id => id).first if has_capability?(:storage_volumes)
>      end
>  
> -    def storage_snapshot(credentials, opts)
> -      storage_snapshots(credentials, opts).first if has_capability?(:storage_snapshots)
> +    def storage_snapshot(credentials, id)
> +      storage_snapshots(credentials, :id => id).first if has_capability?(:storage_snapshots)
>      end
>  
> -    def bucket(credentials, opts = {})
> +    def bucket(credentials, id)
>        #list of objects within bucket
> -      buckets(credentials, opts).first if has_capability?(:buckets)
> +      buckets(credentials, :id => id).first if has_capability?(:buckets)
>      end
>  
> -    def blob(credentials, opts = {})
> -      blobs(credentials, opts).first if has_capability?(:blobs)
> +    def blob(credentials, id)
> +      blobs(credentials, :id => id).first if has_capability?(:blobs)
>      end
>  
> -    def key(credentials, opts=nil)
> -      keys(credentials, opts).first if has_capability?(:keys)
> +    def key(credentials, id)
> +      keys(credentials, :id => id).first if has_capability?(:keys)
>      end
>  
> -    def firewall(credentials, opts={})
> -      firewalls(credentials, opts).first if has_capability?(:firewalls)
> +    def firewall(credentials, id)
> +      firewalls(credentials, :id => id).first if has_capability?(:firewalls)
>      end
>  
>      MEMBER_SHOW_METHODS =
> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> index b55132e..2fff0f9 100644
> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> @@ -183,12 +183,12 @@ module Deltacloud
>            end
>          end
>  
> -        def instance(credentials, opts={})
> +        def instance(credentials, id)
>            ec2 = new_client(credentials)
>            inst_arr = []
>            safely do
> -            ec2_inst = ec2.describe_instances([opts[:id]]).first
> -            raise "Instance #{opts[:id]} NotFound" if ec2_inst.nil?
> +            ec2_inst = ec2.describe_instances([id]).first
> +            raise "Instance #{id} NotFound" if ec2_inst.nil?
>              instance = convert_instance(ec2_inst)
>              return nil unless instance
>              if ec2_inst[:aws_platform] == 'windows'
> @@ -265,7 +265,7 @@ module Deltacloud
>          def reboot_instance(credentials, instance_id)
>            ec2 = new_client(credentials)
>            if ec2.reboot_instances([instance_id])
> -            instance(credentials, :id => instance_id)
> +            instance(credentials, instance_id)
>            else
>              raise Deltacloud::BackendError.new(500, "Instance", "Instance reboot failed", "")
>            end
> @@ -274,7 +274,7 @@ module Deltacloud
>          def destroy_instance(credentials, instance_id)
>            ec2 = new_client(credentials)
>            if ec2.terminate_instances([instance_id])
> -            instance(credentials, :id => instance_id)
> +            instance(credentials, instance_id)
>            else
>              raise Deltacloud::BackendError.new(500, "Instance", "Instance cannot be terminated", "")
>            end
> diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> index 4d65e7c..0edfc39 100644
> --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> @@ -822,6 +822,7 @@ end
>    ######################################################################
>    # Firewalls
>    ######################################################################
> +
>    def firewalls(credentials, opts={})
>      firewalls = []
>      fw_name = 'Firewall' # currently always 'Firewall'
> diff --git a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> index 6469312..c87c65b 100644
> --- a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> +++ b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> @@ -63,7 +63,7 @@ class GogridDriver < Deltacloud::BaseDriver
>      @hardware_profiles
>    end
>  
> -  def images(credentials, opts=nil)
> +  def images(credentials, opts={})
>      imgs = []
>      if opts and opts[:id]
>        safely do
> @@ -80,7 +80,7 @@ class GogridDriver < Deltacloud::BaseDriver
>      imgs.sort_by{|e| [e.owner_id, e.description]}
>    end
>  
> -  def realms(credentials, opts=nil)
> +  def realms(credentials, opts={})
>      realms = []
>      client = new_client(credentials)
>      safely do
> @@ -163,7 +163,7 @@ class GogridDriver < Deltacloud::BaseDriver
>      instances
>    end
>  
> -  def instances(credentials, opts=nil)
> +  def instances(credentials, opts={})
>      instances = []
>      if opts and opts[:id]
>        begin
> @@ -298,11 +298,11 @@ class GogridDriver < Deltacloud::BaseDriver
>      'Unregistering instances from load balancer is not supported in GoGrid')
>    end
>  
> -  def key(credentials, opts=nil)
> -    keys(credentials, opts).first
> +  def key(credentials, id)
> +    keys(credentials, :id => id).first
>    end
>  
> -  def keys(credentials, opts=nil)
> +  def keys(credentials, opts={})
>      gogrid = new_client( credentials )
>      creds = []
>      safely do
> diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb b/server/lib/deltacloud/drivers/mock/mock_driver.rb
> index ba817c6..6ea8764 100644
> --- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
> +++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
> @@ -90,7 +90,7 @@ module Deltacloud::Drivers::Mock
>        @client = Client.new(storage_root)
>      end
>  
> -    def realms(credentials, opts=nil)
> +    def realms(credentials, opts={})
>        check_credentials( credentials )
>        results = []
>        safely do
> @@ -110,7 +110,7 @@ module Deltacloud::Drivers::Mock
>      #
>      # Images
>      #
> -    def images(credentials, opts=nil )
> +    def images(credentials, opts={} )
>        check_credentials( credentials )
>        images = []
>        images = @client.build_all(Image)
> @@ -152,14 +152,14 @@ module Deltacloud::Drivers::Mock
>      # Instances
>      #
>  
> -    def instance(credentials, opts={})
> +    def instance(credentials, id)
>        check_credentials( credentials )
> -      if instance = @client.load(:instances, opts[:id])
> +      if instance = @client.load(:instances, id)
>          Instance.new(instance)
>        end
>      end
>  
> -    def instances(credentials, opts=nil)
> +    def instances(credentials, opts={})
>        check_credentials( credentials )
>        instances = @client.build_all(Instance)
>        instances = filter_on( instances, :owner_id, :owner_id => credentials.user )
> @@ -250,14 +250,14 @@ module Deltacloud::Drivers::Mock
>      #
>      # Storage Volumes
>      #
> -    def storage_volumes(credentials, opts=nil)
> +    def storage_volumes(credentials, opts={})
>        check_credentials( credentials )
>        volumes = @client.build_all(StorageVolume)
>        volumes = filter_on( volumes, :id, opts )
>        volumes
>      end
>  
> -    def create_storage_volume(credentials, opts=nil)
> +    def create_storage_volume(credentials, opts={})
>        check_credentials(credentials)
>        opts ||= {}
>        opts[:capacity] ||= "1"
> @@ -273,18 +273,18 @@ module Deltacloud::Drivers::Mock
>        StorageVolume.new(volume)
>      end
>  
> -    def destroy_storage_volume(credentials, opts=nil)
> +    def destroy_storage_volume(credentials, id)
>        check_credentials(credentials)
> -      @client.destroy(:storage_volumes, opts[:id])
> +      @client.destroy(:storage_volumes, id)
>      end
>  
>      #opts: {:id=,:instance_id,:device}
> -    def attach_storage_volume(credentials, opts)
> +    def attach_storage_volume(credentials, opts={})
>        check_credentials(credentials)
>        attach_volume_instance(opts[:id], opts[:device], opts[:instance_id])
>      end
>  
> -    def detach_storage_volume(credentials, opts)
> +    def detach_storage_volume(credentials, opts={})
>        check_credentials(credentials)
>        detach_volume_instance(opts[:id], opts[:instance_id])
>      end
> @@ -293,7 +293,7 @@ module Deltacloud::Drivers::Mock
>      # Storage Snapshots
>      #
>  
> -    def storage_snapshots(credentials, opts=nil)
> +    def storage_snapshots(credentials, opts={})
>        check_credentials( credentials )
>        snapshots = @client.build_all(StorageSnapshot)
>        snapshots = filter_on(snapshots, :id, opts )
> @@ -307,10 +307,6 @@ module Deltacloud::Drivers::Mock
>        result
>      end
>  
> -    def key(credentials, opts={})
> -      keys(credentials, opts).first
> -    end
> -
>      def create_key(credentials, opts={})
>        check_credentials(credentials)
>        key_hash = {
> diff --git a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> index cc1f13e..7acaf0d 100644
> --- a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> +++ b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> @@ -32,7 +32,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>    ######################################################################
>    # Hardware profiles
>    #####################################################################
> -  def hardware_profiles(credentials, opts=nil)
> +  def hardware_profiles(credentials, opts={})
>      occi_client = new_client(credentials)
>      xml = occi_client.get_instance_types
>      if CloudClient.is_error?(xml)
> @@ -70,8 +70,8 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>    ] ) unless defined?( REALMS )
>  
>  
> -  def realms(credentials, opts=nil)
> -    return REALMS if ( opts.nil? )
> +  def realms(credentials, opts={})
> +    return REALMS if ( opts.keys.empty? )
>      results = REALMS
>      results = filter_on( results, :id, opts )
>      results
> @@ -81,7 +81,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>    ######################################################################
>    # Images
>    ######################################################################
> -  def images(credentials, opts=nil)
> +  def images(credentials, opts={})
>      occi_client = new_client(credentials)
>  
>      xml = treat_response(occi_client.get_images)
> @@ -94,9 +94,9 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>      end
>    end
>  
> -  def image(credentials, opts=nil)
> +  def image(credentials, id)
>      occi_client = new_client(credentials)
> -    xml = treat_response(occi_client.get_image(opts[:id]))
> +    xml = treat_response(occi_client.get_image(id))
>      convert_image(xml, credentials)
>    end
>  
> @@ -153,7 +153,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>      stopping.to(:finish)        .automatically
>    end
>  
> -  def instances(credentials, opts=nil)
> +  def instances(credentials, opts={})
>      occi_client = new_client(credentials)
>  
>      xml = treat_response(occi_client.get_vms)
> @@ -168,13 +168,13 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>      instances = filter_on( instances, :state, opts )
>    end
>  
> -  def instance(credentials, opts=nil)
> +  def instance(credentials, id)
>      occi_client = new_client(credentials)
> -    xml = treat_response(occi_client.get_vm(opts[:id]))
> +    xml = treat_response(occi_client.get_vm(id))
>      convert_instance(xml, credentials)
>    end
>  
> -  def create_instance(credentials, image_id, opts=nil)
> +  def create_instance(credentials, image_id, opts={})
>      occi_client = new_client(credentials)
>  
>      storage_href = "#{occi_client.endpoint}/storage/#{image_id}"
> diff --git a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> index ca1c236..4042f3e 100644
> --- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> +++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> @@ -46,7 +46,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
>      filter_hardware_profiles(results, opts)
>    end
>  
> -  def images(credentials, opts=nil)
> +  def images(credentials, opts={})
>      rs = new_client(credentials)
>      results = []
>      safely do
> @@ -65,7 +65,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
>    end
>  
>    #rackspace does not at this stage have realms... its all US/TX, all the time (at least at time of writing)
> -  def realms(credentials, opts=nil)
> +  def realms(credentials, opts={})
>      [Realm.new( {
>        :id=>"us",
>        :name=>"United States",
> diff --git a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> index 55eec11..bf6d561 100644
> --- a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> +++ b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> @@ -26,7 +26,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
>  
>    feature :instances, :user_name
>  
> -  def images(credentails, opts=nil)
> +  def images(credentails, opts={})
>      safely do
>        rh = RimuHostingClient.new(credentails)
>        images = rh.list_images.map do | image |
> @@ -61,7 +61,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
>      filter_hardware_profiles(results, opts)
>    end
>  
> -  def realms(credentials, opts=nil)
> +  def realms(credentials, opts={})
>      [Realm.new( {
>              :id=>"rimu",
>              :name=>"RimuHosting",
> @@ -69,7 +69,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
>      } )]
>    end
>  
> -  def instances(credentials, opts=nil)
> +  def instances(credentials, opts={})
>      safely do
>        rh = RimuHostingClient.new(credentials)
>        instances = rh.list_nodes.map do | inst |
> diff --git a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> index 3260c47..ba2fb04 100644
> --- a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> +++ b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> @@ -54,7 +54,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>  "PENDING", "2" =>  "STOPPED", "4"
>  # IMAGES
>  #--
>  #aka "vapp_templates"
> -  def images(credentials, opts=nil)
> +  def images(credentials, opts={})
>        image_list = []
>        terremark_client = new_client(credentials)
>        safely do
> @@ -78,7 +78,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>  "PENDING", "2" =>  "STOPPED", "4"
>  # REALMS
>  #--
>  #only one realm... everything in US?
> -  def realms(credentials, opts=nil)
> +  def realms(credentials, opts={})
>       [Realm.new( {
>        :id=>"US-Miami",
>        :name=>"United States - Miami",
> @@ -90,7 +90,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>  "PENDING", "2" =>  "STOPPED", "4"
>  # INSTANCES
>  #--
>  #aka vApps
> -  def instances(credentials, opts=nil)
> +  def instances(credentials, opts={})
>        instances = []
>        terremark_client = new_client(credentials)
>        safely do
> diff --git a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> index 253a2e5..7b1b1e4 100644
> --- a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> +++ b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> @@ -73,7 +73,7 @@ module Deltacloud::Drivers::Vsphere
>  
>      # Images are virtual machines with 'template' flag set to be true.
>      # Thus we're getting them using find_vm and list_virtual_machines
> -    def images(credentials, opts=nil)
> +    def images(credentials, opts={})
>        cloud = new_client(credentials)
>        img_arr = []
>        profiles = hardware_profiles(credentials)
> @@ -123,7 +123,7 @@ module Deltacloud::Drivers::Vsphere
>      end
>  
>      # List all datacenters managed by the vSphere or vCenter entrypoint.
> -    def realms(credentials, opts=nil)
> +    def realms(credentials, opts={})
>        vsphere = new_client(credentials)
>        safely do
>          if opts and opts[:id]
> @@ -140,7 +140,7 @@ module Deltacloud::Drivers::Vsphere
>  
>      # List all running instances, across all datacenters. DeltaCloud API does
>      # not yet support filtering instances by realm.
> -    def instances(credentials, opts=nil)
> +    def instances(credentials, opts={})
>        cloud = new_client(credentials)
>        inst_arr, machine_vms, pending_vms = [], [], []
>        safely do
> diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> index 4ad0be9..ef107c7 100644
> --- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
> +++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> @@ -79,7 +79,7 @@ module Deltacloud::Helpers
>  
>      def show(model)
>        @benchmark = Benchmark.measure do
> -        @element = driver.send(model, credentials, { :id => params[:id]} )
> +        @element = driver.send(model, credentials, params[:id])
>        end
>        headers['X-Backend-Runtime'] = @benchmark.real.to_s
>        instance_variable_set("@#{model}", @element)
> @@ -105,7 +105,7 @@ module Deltacloud::Helpers
>      end
>  
>      def instance_action(name)
> -      original_instance = driver.instance(credentials, :id => params[:id])
> +      original_instance = driver.instance(credentials, params[:id])
>  
>        # If original instance doesn't include called action
>        # return with 405 error (Method is not Allowed)
> diff --git a/server/tests/api/driver_test.rb b/server/tests/api/driver_test.rb
> index c9facbf..2ec22ef 100644
> --- a/server/tests/api/driver_test.rb
> +++ b/server/tests/api/driver_test.rb
> @@ -47,19 +47,19 @@ describe 'Deltacloud drivers API' do
>  
>    METHODS = {
>      :firewalls => [[:credentials], [:opts, "{  }"]],
> -    :firewall  => [[:credentials], [:opts, "{  }"]],
> +    :firewall  => [[:credentials], [:id,]],
>      :keys    => [[:credentials], [:opts, "{  }"]],
> -    :key     => [[:credentials], [:opts]],
> +    :key     => [[:credentials], [:id]],
>      :storage_snapshots => [[:credentials], [:opts, "{  }"]],
> -    :storage_snapshot  => [[:credentials], [:opts]],
> +    :storage_snapshot  => [[:credentials], [:id]],
>      :storage_volumes => [[:credentials], [:opts, "{  }"]],
> -    :storage_volume  => [[:credentials], [:opts]],
> +    :storage_volume  => [[:credentials], [:id]],
>      :realms    => [[:credentials], [:opts, "{  }"]],
> -    :realm     => [[:credentials], [:opts]],
> +    :realm     => [[:credentials], [:id]],
>      :images    => [[:credentials], [:opts, "{  }"]],
> -    :image     => [[:credentials], [:opts]],
> +    :image     => [[:credentials], [:id]],
>      :instances => [[:credentials], [:opts, "{  }"]],
> -    :instance  => [[:credentials], [:opts]],
> +    :instance  => [[:credentials], [:id]],
>      :create_instance => [[:credentials], [:image_id], [:opts, "{  }"]],
>      :destroy_instance => [[:credentials], [:id]],
>      :stop_instance => [[:credentials], [:id]],
> @@ -71,7 +71,13 @@ describe 'Deltacloud drivers API' do
>      METHODS.each do |m, definition|
>        it "should have the correct parameters for the :#{m} method in #{key} driver" do
>          next unless Deltacloud.new(key).backend.respond_to? m
> -        Arguments.names(Deltacloud.new(key).backend.class, m).must_equal definition
> +        is_optional = (definition[1][1] and definition[1][1] == '{  }')
> +        if is_optional
> +          Arguments.names(Deltacloud.new(key).backend.class, m)[1][1].wont_be_nil
> +          Arguments.names(Deltacloud.new(key).backend.class, m)[1][1].must_equal '{  }'
> +        else
> +          Arguments.names(Deltacloud.new(key).backend.class, m)[1][1].must_be_nil
> +        end
>        end
>      end
>    end
> diff --git a/server/tests/drivers/mock/instances_test.rb b/server/tests/drivers/mock/instances_test.rb
> index d44fac5..0855190 100644
> --- a/server/tests/drivers/mock/instances_test.rb
> +++ b/server/tests/drivers/mock/instances_test.rb
> @@ -261,6 +261,7 @@ describe 'Deltacloud API instances' do
>        :image_id => image_id,
>        :realm_id => realm_id,
>      }
> +    puts last_response.body
>      last_response.status.must_equal 201 # HTTP_CREATED
>      last_response.headers['Location'].wont_be_nil # Location header must be set, pointing to new the instance
>      instance_id = last_response.headers['Location'].split('/').last
> 



RE: [PATCH core] Core: Unified the method parameters across all drivers to have consistent drivers API

Posted by "Koper, Dies" <di...@fast.au.fujitsu.com>.
Hi Michal,

> diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> index 4d65e7c..0edfc39 100644
> --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> @@ -822,6 +822,7 @@ end
> 
> ##########################################################
> ############
>    # Firewalls
> 
> ##########################################################
> ############
> +
>    def firewalls(credentials, opts={})
>      firewalls = []
>      fw_name = 'Firewall' # currently always 'Firewall'

Why?
You haven't touched the method parameters and none of the other comment
blocks in this file have an empty newline here (unless I missed a
patch).

Regards,
Dies Koper



> -----Original Message-----
> From: mfojtik@redhat.com [mailto:mfojtik@redhat.com]
> Sent: Wednesday, 23 May 2012 7:39 AM
> To: dev@deltacloud.apache.org
> Subject: [PATCH core] Core: Unified the method parameters across all
> drivers to have consistent drivers API
> 
> From: Michal Fojtik <mf...@redhat.com>
> 
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
>  server/lib/deltacloud/collections/images.rb        |    2 +-
>  server/lib/deltacloud/collections/keys.rb          |    2 +-
>  server/lib/deltacloud/drivers/base_driver.rb       |   40
++++++++++----------
>  server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |   10 ++---
>  server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb  |    1 +
>  .../lib/deltacloud/drivers/gogrid/gogrid_driver.rb |   12 +++---
>  server/lib/deltacloud/drivers/mock/mock_driver.rb  |   28
++++++--------
>  .../drivers/opennebula/opennebula_driver.rb        |   20 +++++-----
>  .../drivers/rackspace/rackspace_driver.rb          |    4 +-
>  .../drivers/rimuhosting/rimuhosting_driver.rb      |    6 +--
>  .../drivers/terremark/terremark_driver.rb          |    6 +--
>  .../deltacloud/drivers/vsphere/vsphere_driver.rb   |    6 +--
>  server/lib/deltacloud/helpers/deltacloud_helper.rb |    4 +-
>  server/tests/api/driver_test.rb                    |   22 +++++++----
>  server/tests/drivers/mock/instances_test.rb        |    1 +
>  15 files changed, 84 insertions(+), 80 deletions(-)
> 
> diff --git a/server/lib/deltacloud/collections/images.rb
> b/server/lib/deltacloud/collections/images.rb
> index 0edd7c0..85e5918 100644
> --- a/server/lib/deltacloud/collections/images.rb
> +++ b/server/lib/deltacloud/collections/images.rb
> @@ -58,7 +58,7 @@ module Deltacloud::Collections
> 
>        operation :destroy, :with_capability => :destroy_image do
>          control do
> -          driver.destroy_image(credentials, params[:id])
> +          driver.destroy_image(credentials, id)
>            respond_to do |format|
>              format.xml { status 204 }
>              format.json { status 204 }
> diff --git a/server/lib/deltacloud/collections/keys.rb
> b/server/lib/deltacloud/collections/keys.rb
> index 55d0fa8..3be8e00 100644
> --- a/server/lib/deltacloud/collections/keys.rb
> +++ b/server/lib/deltacloud/collections/keys.rb
> @@ -45,7 +45,7 @@ module Deltacloud::Collections
> 
>        operation :destroy, :with_capability => :destroy_key do
>          control do
> -          driver.destroy_key(credentials, { :id => params[:id]})
> +          driver.destroy_key(credentials, params[:id])
>            status 204
>            respond_to do |format|
>              format.xml
> diff --git a/server/lib/deltacloud/drivers/base_driver.rb
> b/server/lib/deltacloud/drivers/base_driver.rb
> index a360807..beb794b 100644
> --- a/server/lib/deltacloud/drivers/base_driver.rb
> +++ b/server/lib/deltacloud/drivers/base_driver.rb
> @@ -115,12 +115,12 @@ module Deltacloud
>      def find_hardware_profile(credentials, name, image_id)
>        hwp = nil
>        if name
> -        unless hwp = hardware_profiles(credentials, :id =>
name).first
> +        unless hwp = hardware_profile(credentials, name)
>            raise BackendError.new(400, "bad-hardware-profile-name",
>              "Hardware profile '#{name}' does not exist", nil)
>          end
>        else
> -        unless image = image(credentials, :id=>image_id)
> +        unless image = image(credentials, image_id)
>            raise BackendError.new(400, "bad-image-id",
>                "Image with ID '#{image_id}' does not exist", nil)
>          end
> @@ -206,41 +206,41 @@ module Deltacloud
>      # def create_firewall_rule(credentials, opts)
>      # def delete_firewall_rule(credentials, opts)
>      # def providers(credentials)
> -    def realm(credentials, opts)
> -      realms = realms(credentials, opts).first if
has_capability?(:realms)
> +    def realm(credentials, id)
> +      realms = realms(credentials, :id => id).first if
has_capability?(:realms)
>      end
> 
> -    def image(credentials, opts)
> -      images(credentials, opts).first if has_capability?(:images)
> +    def image(credentials, id)
> +      images(credentials, :id => id).first if
has_capability?(:images)
>      end
> 
> -    def instance(credentials, opts)
> -      instances(credentials, opts).first if
has_capability?(:instances)
> +    def instance(credentials, id)
> +      instances(credentials, :id => id).first if
has_capability?(:instances)
>      end
> 
> -    def storage_volume(credentials, opts)
> -      storage_volumes(credentials, opts).first if
> has_capability?(:storage_volumes)
> +    def storage_volume(credentials, id)
> +      storage_volumes(credentials, :id => id).first if
> has_capability?(:storage_volumes)
>      end
> 
> -    def storage_snapshot(credentials, opts)
> -      storage_snapshots(credentials, opts).first if
> has_capability?(:storage_snapshots)
> +    def storage_snapshot(credentials, id)
> +      storage_snapshots(credentials, :id => id).first if
> has_capability?(:storage_snapshots)
>      end
> 
> -    def bucket(credentials, opts = {})
> +    def bucket(credentials, id)
>        #list of objects within bucket
> -      buckets(credentials, opts).first if has_capability?(:buckets)
> +      buckets(credentials, :id => id).first if
has_capability?(:buckets)
>      end
> 
> -    def blob(credentials, opts = {})
> -      blobs(credentials, opts).first if has_capability?(:blobs)
> +    def blob(credentials, id)
> +      blobs(credentials, :id => id).first if has_capability?(:blobs)
>      end
> 
> -    def key(credentials, opts=nil)
> -      keys(credentials, opts).first if has_capability?(:keys)
> +    def key(credentials, id)
> +      keys(credentials, :id => id).first if has_capability?(:keys)
>      end
> 
> -    def firewall(credentials, opts={})
> -      firewalls(credentials, opts).first if
has_capability?(:firewalls)
> +    def firewall(credentials, id)
> +      firewalls(credentials, :id => id).first if
has_capability?(:firewalls)
>      end
> 
>      MEMBER_SHOW_METHODS =
> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> index b55132e..2fff0f9 100644
> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> @@ -183,12 +183,12 @@ module Deltacloud
>            end
>          end
> 
> -        def instance(credentials, opts={})
> +        def instance(credentials, id)
>            ec2 = new_client(credentials)
>            inst_arr = []
>            safely do
> -            ec2_inst = ec2.describe_instances([opts[:id]]).first
> -            raise "Instance #{opts[:id]} NotFound" if ec2_inst.nil?
> +            ec2_inst = ec2.describe_instances([id]).first
> +            raise "Instance #{id} NotFound" if ec2_inst.nil?
>              instance = convert_instance(ec2_inst)
>              return nil unless instance
>              if ec2_inst[:aws_platform] == 'windows'
> @@ -265,7 +265,7 @@ module Deltacloud
>          def reboot_instance(credentials, instance_id)
>            ec2 = new_client(credentials)
>            if ec2.reboot_instances([instance_id])
> -            instance(credentials, :id => instance_id)
> +            instance(credentials, instance_id)
>            else
>              raise Deltacloud::BackendError.new(500, "Instance",
"Instance
> reboot failed", "")
>            end
> @@ -274,7 +274,7 @@ module Deltacloud
>          def destroy_instance(credentials, instance_id)
>            ec2 = new_client(credentials)
>            if ec2.terminate_instances([instance_id])
> -            instance(credentials, :id => instance_id)
> +            instance(credentials, instance_id)
>            else
>              raise Deltacloud::BackendError.new(500, "Instance",
"Instance
> cannot be terminated", "")
>            end
> diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> index 4d65e7c..0edfc39 100644
> --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> @@ -822,6 +822,7 @@ end
> 
> ##########################################################
> ############
>    # Firewalls
> 
> ##########################################################
> ############
> +
>    def firewalls(credentials, opts={})
>      firewalls = []
>      fw_name = 'Firewall' # currently always 'Firewall'
> diff --git a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> index 6469312..c87c65b 100644
> --- a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> +++ b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
> @@ -63,7 +63,7 @@ class GogridDriver < Deltacloud::BaseDriver
>      @hardware_profiles
>    end
> 
> -  def images(credentials, opts=nil)
> +  def images(credentials, opts={})
>      imgs = []
>      if opts and opts[:id]
>        safely do
> @@ -80,7 +80,7 @@ class GogridDriver < Deltacloud::BaseDriver
>      imgs.sort_by{|e| [e.owner_id, e.description]}
>    end
> 
> -  def realms(credentials, opts=nil)
> +  def realms(credentials, opts={})
>      realms = []
>      client = new_client(credentials)
>      safely do
> @@ -163,7 +163,7 @@ class GogridDriver < Deltacloud::BaseDriver
>      instances
>    end
> 
> -  def instances(credentials, opts=nil)
> +  def instances(credentials, opts={})
>      instances = []
>      if opts and opts[:id]
>        begin
> @@ -298,11 +298,11 @@ class GogridDriver < Deltacloud::BaseDriver
>      'Unregistering instances from load balancer is not supported in
GoGrid')
>    end
> 
> -  def key(credentials, opts=nil)
> -    keys(credentials, opts).first
> +  def key(credentials, id)
> +    keys(credentials, :id => id).first
>    end
> 
> -  def keys(credentials, opts=nil)
> +  def keys(credentials, opts={})
>      gogrid = new_client( credentials )
>      creds = []
>      safely do
> diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb
> b/server/lib/deltacloud/drivers/mock/mock_driver.rb
> index ba817c6..6ea8764 100644
> --- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
> +++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
> @@ -90,7 +90,7 @@ module Deltacloud::Drivers::Mock
>        @client = Client.new(storage_root)
>      end
> 
> -    def realms(credentials, opts=nil)
> +    def realms(credentials, opts={})
>        check_credentials( credentials )
>        results = []
>        safely do
> @@ -110,7 +110,7 @@ module Deltacloud::Drivers::Mock
>      #
>      # Images
>      #
> -    def images(credentials, opts=nil )
> +    def images(credentials, opts={} )
>        check_credentials( credentials )
>        images = []
>        images = @client.build_all(Image)
> @@ -152,14 +152,14 @@ module Deltacloud::Drivers::Mock
>      # Instances
>      #
> 
> -    def instance(credentials, opts={})
> +    def instance(credentials, id)
>        check_credentials( credentials )
> -      if instance = @client.load(:instances, opts[:id])
> +      if instance = @client.load(:instances, id)
>          Instance.new(instance)
>        end
>      end
> 
> -    def instances(credentials, opts=nil)
> +    def instances(credentials, opts={})
>        check_credentials( credentials )
>        instances = @client.build_all(Instance)
>        instances = filter_on( instances, :owner_id, :owner_id =>
> credentials.user )
> @@ -250,14 +250,14 @@ module Deltacloud::Drivers::Mock
>      #
>      # Storage Volumes
>      #
> -    def storage_volumes(credentials, opts=nil)
> +    def storage_volumes(credentials, opts={})
>        check_credentials( credentials )
>        volumes = @client.build_all(StorageVolume)
>        volumes = filter_on( volumes, :id, opts )
>        volumes
>      end
> 
> -    def create_storage_volume(credentials, opts=nil)
> +    def create_storage_volume(credentials, opts={})
>        check_credentials(credentials)
>        opts ||= {}
>        opts[:capacity] ||= "1"
> @@ -273,18 +273,18 @@ module Deltacloud::Drivers::Mock
>        StorageVolume.new(volume)
>      end
> 
> -    def destroy_storage_volume(credentials, opts=nil)
> +    def destroy_storage_volume(credentials, id)
>        check_credentials(credentials)
> -      @client.destroy(:storage_volumes, opts[:id])
> +      @client.destroy(:storage_volumes, id)
>      end
> 
>      #opts: {:id=,:instance_id,:device}
> -    def attach_storage_volume(credentials, opts)
> +    def attach_storage_volume(credentials, opts={})
>        check_credentials(credentials)
>        attach_volume_instance(opts[:id], opts[:device],
opts[:instance_id])
>      end
> 
> -    def detach_storage_volume(credentials, opts)
> +    def detach_storage_volume(credentials, opts={})
>        check_credentials(credentials)
>        detach_volume_instance(opts[:id], opts[:instance_id])
>      end
> @@ -293,7 +293,7 @@ module Deltacloud::Drivers::Mock
>      # Storage Snapshots
>      #
> 
> -    def storage_snapshots(credentials, opts=nil)
> +    def storage_snapshots(credentials, opts={})
>        check_credentials( credentials )
>        snapshots = @client.build_all(StorageSnapshot)
>        snapshots = filter_on(snapshots, :id, opts )
> @@ -307,10 +307,6 @@ module Deltacloud::Drivers::Mock
>        result
>      end
> 
> -    def key(credentials, opts={})
> -      keys(credentials, opts).first
> -    end
> -
>      def create_key(credentials, opts={})
>        check_credentials(credentials)
>        key_hash = {
> diff --git
a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> index cc1f13e..7acaf0d 100644
> --- a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> +++ b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
> @@ -32,7 +32,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
> 
> ##########################################################
> ############
>    # Hardware profiles
> 
> ##########################################################
> ###########
> -  def hardware_profiles(credentials, opts=nil)
> +  def hardware_profiles(credentials, opts={})
>      occi_client = new_client(credentials)
>      xml = occi_client.get_instance_types
>      if CloudClient.is_error?(xml)
> @@ -70,8 +70,8 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>    ] ) unless defined?( REALMS )
> 
> 
> -  def realms(credentials, opts=nil)
> -    return REALMS if ( opts.nil? )
> +  def realms(credentials, opts={})
> +    return REALMS if ( opts.keys.empty? )
>      results = REALMS
>      results = filter_on( results, :id, opts )
>      results
> @@ -81,7 +81,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
> 
> ##########################################################
> ############
>    # Images
> 
> ##########################################################
> ############
> -  def images(credentials, opts=nil)
> +  def images(credentials, opts={})
>      occi_client = new_client(credentials)
> 
>      xml = treat_response(occi_client.get_images)
> @@ -94,9 +94,9 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>      end
>    end
> 
> -  def image(credentials, opts=nil)
> +  def image(credentials, id)
>      occi_client = new_client(credentials)
> -    xml = treat_response(occi_client.get_image(opts[:id]))
> +    xml = treat_response(occi_client.get_image(id))
>      convert_image(xml, credentials)
>    end
> 
> @@ -153,7 +153,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>      stopping.to(:finish)        .automatically
>    end
> 
> -  def instances(credentials, opts=nil)
> +  def instances(credentials, opts={})
>      occi_client = new_client(credentials)
> 
>      xml = treat_response(occi_client.get_vms)
> @@ -168,13 +168,13 @@ class OpennebulaDriver < Deltacloud::BaseDriver
>      instances = filter_on( instances, :state, opts )
>    end
> 
> -  def instance(credentials, opts=nil)
> +  def instance(credentials, id)
>      occi_client = new_client(credentials)
> -    xml = treat_response(occi_client.get_vm(opts[:id]))
> +    xml = treat_response(occi_client.get_vm(id))
>      convert_instance(xml, credentials)
>    end
> 
> -  def create_instance(credentials, image_id, opts=nil)
> +  def create_instance(credentials, image_id, opts={})
>      occi_client = new_client(credentials)
> 
>      storage_href = "#{occi_client.endpoint}/storage/#{image_id}"
> diff --git
a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> index ca1c236..4042f3e 100644
> --- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> +++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
> @@ -46,7 +46,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
>      filter_hardware_profiles(results, opts)
>    end
> 
> -  def images(credentials, opts=nil)
> +  def images(credentials, opts={})
>      rs = new_client(credentials)
>      results = []
>      safely do
> @@ -65,7 +65,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
>    end
> 
>    #rackspace does not at this stage have realms... its all US/TX, all
the time
> (at least at time of writing)
> -  def realms(credentials, opts=nil)
> +  def realms(credentials, opts={})
>      [Realm.new( {
>        :id=>"us",
>        :name=>"United States",
> diff --git
a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> index 55eec11..bf6d561 100644
> --- a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> +++ b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
> @@ -26,7 +26,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
> 
>    feature :instances, :user_name
> 
> -  def images(credentails, opts=nil)
> +  def images(credentails, opts={})
>      safely do
>        rh = RimuHostingClient.new(credentails)
>        images = rh.list_images.map do | image |
> @@ -61,7 +61,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
>      filter_hardware_profiles(results, opts)
>    end
> 
> -  def realms(credentials, opts=nil)
> +  def realms(credentials, opts={})
>      [Realm.new( {
>              :id=>"rimu",
>              :name=>"RimuHosting",
> @@ -69,7 +69,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
>      } )]
>    end
> 
> -  def instances(credentials, opts=nil)
> +  def instances(credentials, opts={})
>      safely do
>        rh = RimuHostingClient.new(credentials)
>        instances = rh.list_nodes.map do | inst |
> diff --git
a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> index 3260c47..ba2fb04 100644
> --- a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> +++ b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
> @@ -54,7 +54,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
> "PENDING", "2" =>  "STOPPED", "4"
>  # IMAGES
>  #--
>  #aka "vapp_templates"
> -  def images(credentials, opts=nil)
> +  def images(credentials, opts={})
>        image_list = []
>        terremark_client = new_client(credentials)
>        safely do
> @@ -78,7 +78,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
> "PENDING", "2" =>  "STOPPED", "4"
>  # REALMS
>  #--
>  #only one realm... everything in US?
> -  def realms(credentials, opts=nil)
> +  def realms(credentials, opts={})
>       [Realm.new( {
>        :id=>"US-Miami",
>        :name=>"United States - Miami",
> @@ -90,7 +90,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>
> "PENDING", "2" =>  "STOPPED", "4"
>  # INSTANCES
>  #--
>  #aka vApps
> -  def instances(credentials, opts=nil)
> +  def instances(credentials, opts={})
>        instances = []
>        terremark_client = new_client(credentials)
>        safely do
> diff --git a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> index 253a2e5..7b1b1e4 100644
> --- a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> +++ b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
> @@ -73,7 +73,7 @@ module Deltacloud::Drivers::Vsphere
> 
>      # Images are virtual machines with 'template' flag set to be
true.
>      # Thus we're getting them using find_vm and list_virtual_machines
> -    def images(credentials, opts=nil)
> +    def images(credentials, opts={})
>        cloud = new_client(credentials)
>        img_arr = []
>        profiles = hardware_profiles(credentials)
> @@ -123,7 +123,7 @@ module Deltacloud::Drivers::Vsphere
>      end
> 
>      # List all datacenters managed by the vSphere or vCenter
entrypoint.
> -    def realms(credentials, opts=nil)
> +    def realms(credentials, opts={})
>        vsphere = new_client(credentials)
>        safely do
>          if opts and opts[:id]
> @@ -140,7 +140,7 @@ module Deltacloud::Drivers::Vsphere
> 
>      # List all running instances, across all datacenters. DeltaCloud
API does
>      # not yet support filtering instances by realm.
> -    def instances(credentials, opts=nil)
> +    def instances(credentials, opts={})
>        cloud = new_client(credentials)
>        inst_arr, machine_vms, pending_vms = [], [], []
>        safely do
> diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb
> b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> index 4ad0be9..ef107c7 100644
> --- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
> +++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> @@ -79,7 +79,7 @@ module Deltacloud::Helpers
> 
>      def show(model)
>        @benchmark = Benchmark.measure do
> -        @element = driver.send(model, credentials, { :id =>
params[:id]} )
> +        @element = driver.send(model, credentials, params[:id])
>        end
>        headers['X-Backend-Runtime'] = @benchmark.real.to_s
>        instance_variable_set("@#{model}", @element)
> @@ -105,7 +105,7 @@ module Deltacloud::Helpers
>      end
> 
>      def instance_action(name)
> -      original_instance = driver.instance(credentials, :id =>
params[:id])
> +      original_instance = driver.instance(credentials, params[:id])
> 
>        # If original instance doesn't include called action
>        # return with 405 error (Method is not Allowed)
> diff --git a/server/tests/api/driver_test.rb
b/server/tests/api/driver_test.rb
> index c9facbf..2ec22ef 100644
> --- a/server/tests/api/driver_test.rb
> +++ b/server/tests/api/driver_test.rb
> @@ -47,19 +47,19 @@ describe 'Deltacloud drivers API' do
> 
>    METHODS = {
>      :firewalls => [[:credentials], [:opts, "{  }"]],
> -    :firewall  => [[:credentials], [:opts, "{  }"]],
> +    :firewall  => [[:credentials], [:id,]],
>      :keys    => [[:credentials], [:opts, "{  }"]],
> -    :key     => [[:credentials], [:opts]],
> +    :key     => [[:credentials], [:id]],
>      :storage_snapshots => [[:credentials], [:opts, "{  }"]],
> -    :storage_snapshot  => [[:credentials], [:opts]],
> +    :storage_snapshot  => [[:credentials], [:id]],
>      :storage_volumes => [[:credentials], [:opts, "{  }"]],
> -    :storage_volume  => [[:credentials], [:opts]],
> +    :storage_volume  => [[:credentials], [:id]],
>      :realms    => [[:credentials], [:opts, "{  }"]],
> -    :realm     => [[:credentials], [:opts]],
> +    :realm     => [[:credentials], [:id]],
>      :images    => [[:credentials], [:opts, "{  }"]],
> -    :image     => [[:credentials], [:opts]],
> +    :image     => [[:credentials], [:id]],
>      :instances => [[:credentials], [:opts, "{  }"]],
> -    :instance  => [[:credentials], [:opts]],
> +    :instance  => [[:credentials], [:id]],
>      :create_instance => [[:credentials], [:image_id], [:opts, "{
}"]],
>      :destroy_instance => [[:credentials], [:id]],
>      :stop_instance => [[:credentials], [:id]],
> @@ -71,7 +71,13 @@ describe 'Deltacloud drivers API' do
>      METHODS.each do |m, definition|
>        it "should have the correct parameters for the :#{m} method in
#{key}
> driver" do
>          next unless Deltacloud.new(key).backend.respond_to? m
> -        Arguments.names(Deltacloud.new(key).backend.class,
m).must_equal
> definition
> +        is_optional = (definition[1][1] and definition[1][1] == '{
}')
> +        if is_optional
> +          Arguments.names(Deltacloud.new(key).backend.class,
> m)[1][1].wont_be_nil
> +          Arguments.names(Deltacloud.new(key).backend.class,
> m)[1][1].must_equal '{  }'
> +        else
> +          Arguments.names(Deltacloud.new(key).backend.class,
> m)[1][1].must_be_nil
> +        end
>        end
>      end
>    end
> diff --git a/server/tests/drivers/mock/instances_test.rb
> b/server/tests/drivers/mock/instances_test.rb
> index d44fac5..0855190 100644
> --- a/server/tests/drivers/mock/instances_test.rb
> +++ b/server/tests/drivers/mock/instances_test.rb
> @@ -261,6 +261,7 @@ describe 'Deltacloud API instances' do
>        :image_id => image_id,
>        :realm_id => realm_id,
>      }
> +    puts last_response.body
>      last_response.status.must_equal 201 # HTTP_CREATED
>      last_response.headers['Location'].wont_be_nil # Location header
must
> be set, pointing to new the instance
>      instance_id = last_response.headers['Location'].split('/').last
> --
> 1.7.10.2
> 



[PATCH core] Core: Unified the method parameters across all drivers to have consistent drivers API

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>


Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/deltacloud/collections/images.rb        |    2 +-
 server/lib/deltacloud/collections/keys.rb          |    2 +-
 server/lib/deltacloud/drivers/base_driver.rb       |   40 ++++++++++----------
 server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |   10 ++---
 server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb  |    1 +
 .../lib/deltacloud/drivers/gogrid/gogrid_driver.rb |   12 +++---
 server/lib/deltacloud/drivers/mock/mock_driver.rb  |   28 ++++++--------
 .../drivers/opennebula/opennebula_driver.rb        |   20 +++++-----
 .../drivers/rackspace/rackspace_driver.rb          |    4 +-
 .../drivers/rimuhosting/rimuhosting_driver.rb      |    6 +--
 .../drivers/terremark/terremark_driver.rb          |    6 +--
 .../deltacloud/drivers/vsphere/vsphere_driver.rb   |    6 +--
 server/lib/deltacloud/helpers/deltacloud_helper.rb |    4 +-
 server/tests/api/driver_test.rb                    |   22 +++++++----
 server/tests/drivers/mock/instances_test.rb        |    1 +
 15 files changed, 84 insertions(+), 80 deletions(-)

diff --git a/server/lib/deltacloud/collections/images.rb b/server/lib/deltacloud/collections/images.rb
index 0edd7c0..85e5918 100644
--- a/server/lib/deltacloud/collections/images.rb
+++ b/server/lib/deltacloud/collections/images.rb
@@ -58,7 +58,7 @@ module Deltacloud::Collections
 
       operation :destroy, :with_capability => :destroy_image do
         control do
-          driver.destroy_image(credentials, params[:id])
+          driver.destroy_image(credentials, id)
           respond_to do |format|
             format.xml { status 204 }
             format.json { status 204 }
diff --git a/server/lib/deltacloud/collections/keys.rb b/server/lib/deltacloud/collections/keys.rb
index 55d0fa8..3be8e00 100644
--- a/server/lib/deltacloud/collections/keys.rb
+++ b/server/lib/deltacloud/collections/keys.rb
@@ -45,7 +45,7 @@ module Deltacloud::Collections
 
       operation :destroy, :with_capability => :destroy_key do
         control do
-          driver.destroy_key(credentials, { :id => params[:id]})
+          driver.destroy_key(credentials, params[:id])
           status 204
           respond_to do |format|
             format.xml
diff --git a/server/lib/deltacloud/drivers/base_driver.rb b/server/lib/deltacloud/drivers/base_driver.rb
index a360807..beb794b 100644
--- a/server/lib/deltacloud/drivers/base_driver.rb
+++ b/server/lib/deltacloud/drivers/base_driver.rb
@@ -115,12 +115,12 @@ module Deltacloud
     def find_hardware_profile(credentials, name, image_id)
       hwp = nil
       if name
-        unless hwp = hardware_profiles(credentials, :id => name).first
+        unless hwp = hardware_profile(credentials, name)
           raise BackendError.new(400, "bad-hardware-profile-name",
             "Hardware profile '#{name}' does not exist", nil)
         end
       else
-        unless image = image(credentials, :id=>image_id)
+        unless image = image(credentials, image_id)
           raise BackendError.new(400, "bad-image-id",
               "Image with ID '#{image_id}' does not exist", nil)
         end
@@ -206,41 +206,41 @@ module Deltacloud
     # def create_firewall_rule(credentials, opts)
     # def delete_firewall_rule(credentials, opts)
     # def providers(credentials)
-    def realm(credentials, opts)
-      realms = realms(credentials, opts).first if has_capability?(:realms)
+    def realm(credentials, id)
+      realms = realms(credentials, :id => id).first if has_capability?(:realms)
     end
 
-    def image(credentials, opts)
-      images(credentials, opts).first if has_capability?(:images)
+    def image(credentials, id)
+      images(credentials, :id => id).first if has_capability?(:images)
     end
 
-    def instance(credentials, opts)
-      instances(credentials, opts).first if has_capability?(:instances)
+    def instance(credentials, id)
+      instances(credentials, :id => id).first if has_capability?(:instances)
     end
 
-    def storage_volume(credentials, opts)
-      storage_volumes(credentials, opts).first if has_capability?(:storage_volumes)
+    def storage_volume(credentials, id)
+      storage_volumes(credentials, :id => id).first if has_capability?(:storage_volumes)
     end
 
-    def storage_snapshot(credentials, opts)
-      storage_snapshots(credentials, opts).first if has_capability?(:storage_snapshots)
+    def storage_snapshot(credentials, id)
+      storage_snapshots(credentials, :id => id).first if has_capability?(:storage_snapshots)
     end
 
-    def bucket(credentials, opts = {})
+    def bucket(credentials, id)
       #list of objects within bucket
-      buckets(credentials, opts).first if has_capability?(:buckets)
+      buckets(credentials, :id => id).first if has_capability?(:buckets)
     end
 
-    def blob(credentials, opts = {})
-      blobs(credentials, opts).first if has_capability?(:blobs)
+    def blob(credentials, id)
+      blobs(credentials, :id => id).first if has_capability?(:blobs)
     end
 
-    def key(credentials, opts=nil)
-      keys(credentials, opts).first if has_capability?(:keys)
+    def key(credentials, id)
+      keys(credentials, :id => id).first if has_capability?(:keys)
     end
 
-    def firewall(credentials, opts={})
-      firewalls(credentials, opts).first if has_capability?(:firewalls)
+    def firewall(credentials, id)
+      firewalls(credentials, :id => id).first if has_capability?(:firewalls)
     end
 
     MEMBER_SHOW_METHODS =
diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
index b55132e..2fff0f9 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
@@ -183,12 +183,12 @@ module Deltacloud
           end
         end
 
-        def instance(credentials, opts={})
+        def instance(credentials, id)
           ec2 = new_client(credentials)
           inst_arr = []
           safely do
-            ec2_inst = ec2.describe_instances([opts[:id]]).first
-            raise "Instance #{opts[:id]} NotFound" if ec2_inst.nil?
+            ec2_inst = ec2.describe_instances([id]).first
+            raise "Instance #{id} NotFound" if ec2_inst.nil?
             instance = convert_instance(ec2_inst)
             return nil unless instance
             if ec2_inst[:aws_platform] == 'windows'
@@ -265,7 +265,7 @@ module Deltacloud
         def reboot_instance(credentials, instance_id)
           ec2 = new_client(credentials)
           if ec2.reboot_instances([instance_id])
-            instance(credentials, :id => instance_id)
+            instance(credentials, instance_id)
           else
             raise Deltacloud::BackendError.new(500, "Instance", "Instance reboot failed", "")
           end
@@ -274,7 +274,7 @@ module Deltacloud
         def destroy_instance(credentials, instance_id)
           ec2 = new_client(credentials)
           if ec2.terminate_instances([instance_id])
-            instance(credentials, :id => instance_id)
+            instance(credentials, instance_id)
           else
             raise Deltacloud::BackendError.new(500, "Instance", "Instance cannot be terminated", "")
           end
diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
index 4d65e7c..0edfc39 100644
--- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
+++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
@@ -822,6 +822,7 @@ end
   ######################################################################
   # Firewalls
   ######################################################################
+
   def firewalls(credentials, opts={})
     firewalls = []
     fw_name = 'Firewall' # currently always 'Firewall'
diff --git a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
index 6469312..c87c65b 100644
--- a/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
+++ b/server/lib/deltacloud/drivers/gogrid/gogrid_driver.rb
@@ -63,7 +63,7 @@ class GogridDriver < Deltacloud::BaseDriver
     @hardware_profiles
   end
 
-  def images(credentials, opts=nil)
+  def images(credentials, opts={})
     imgs = []
     if opts and opts[:id]
       safely do
@@ -80,7 +80,7 @@ class GogridDriver < Deltacloud::BaseDriver
     imgs.sort_by{|e| [e.owner_id, e.description]}
   end
 
-  def realms(credentials, opts=nil)
+  def realms(credentials, opts={})
     realms = []
     client = new_client(credentials)
     safely do
@@ -163,7 +163,7 @@ class GogridDriver < Deltacloud::BaseDriver
     instances
   end
 
-  def instances(credentials, opts=nil)
+  def instances(credentials, opts={})
     instances = []
     if opts and opts[:id]
       begin
@@ -298,11 +298,11 @@ class GogridDriver < Deltacloud::BaseDriver
     'Unregistering instances from load balancer is not supported in GoGrid')
   end
 
-  def key(credentials, opts=nil)
-    keys(credentials, opts).first
+  def key(credentials, id)
+    keys(credentials, :id => id).first
   end
 
-  def keys(credentials, opts=nil)
+  def keys(credentials, opts={})
     gogrid = new_client( credentials )
     creds = []
     safely do
diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb b/server/lib/deltacloud/drivers/mock/mock_driver.rb
index ba817c6..6ea8764 100644
--- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
+++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
@@ -90,7 +90,7 @@ module Deltacloud::Drivers::Mock
       @client = Client.new(storage_root)
     end
 
-    def realms(credentials, opts=nil)
+    def realms(credentials, opts={})
       check_credentials( credentials )
       results = []
       safely do
@@ -110,7 +110,7 @@ module Deltacloud::Drivers::Mock
     #
     # Images
     #
-    def images(credentials, opts=nil )
+    def images(credentials, opts={} )
       check_credentials( credentials )
       images = []
       images = @client.build_all(Image)
@@ -152,14 +152,14 @@ module Deltacloud::Drivers::Mock
     # Instances
     #
 
-    def instance(credentials, opts={})
+    def instance(credentials, id)
       check_credentials( credentials )
-      if instance = @client.load(:instances, opts[:id])
+      if instance = @client.load(:instances, id)
         Instance.new(instance)
       end
     end
 
-    def instances(credentials, opts=nil)
+    def instances(credentials, opts={})
       check_credentials( credentials )
       instances = @client.build_all(Instance)
       instances = filter_on( instances, :owner_id, :owner_id => credentials.user )
@@ -250,14 +250,14 @@ module Deltacloud::Drivers::Mock
     #
     # Storage Volumes
     #
-    def storage_volumes(credentials, opts=nil)
+    def storage_volumes(credentials, opts={})
       check_credentials( credentials )
       volumes = @client.build_all(StorageVolume)
       volumes = filter_on( volumes, :id, opts )
       volumes
     end
 
-    def create_storage_volume(credentials, opts=nil)
+    def create_storage_volume(credentials, opts={})
       check_credentials(credentials)
       opts ||= {}
       opts[:capacity] ||= "1"
@@ -273,18 +273,18 @@ module Deltacloud::Drivers::Mock
       StorageVolume.new(volume)
     end
 
-    def destroy_storage_volume(credentials, opts=nil)
+    def destroy_storage_volume(credentials, id)
       check_credentials(credentials)
-      @client.destroy(:storage_volumes, opts[:id])
+      @client.destroy(:storage_volumes, id)
     end
 
     #opts: {:id=,:instance_id,:device}
-    def attach_storage_volume(credentials, opts)
+    def attach_storage_volume(credentials, opts={})
       check_credentials(credentials)
       attach_volume_instance(opts[:id], opts[:device], opts[:instance_id])
     end
 
-    def detach_storage_volume(credentials, opts)
+    def detach_storage_volume(credentials, opts={})
       check_credentials(credentials)
       detach_volume_instance(opts[:id], opts[:instance_id])
     end
@@ -293,7 +293,7 @@ module Deltacloud::Drivers::Mock
     # Storage Snapshots
     #
 
-    def storage_snapshots(credentials, opts=nil)
+    def storage_snapshots(credentials, opts={})
       check_credentials( credentials )
       snapshots = @client.build_all(StorageSnapshot)
       snapshots = filter_on(snapshots, :id, opts )
@@ -307,10 +307,6 @@ module Deltacloud::Drivers::Mock
       result
     end
 
-    def key(credentials, opts={})
-      keys(credentials, opts).first
-    end
-
     def create_key(credentials, opts={})
       check_credentials(credentials)
       key_hash = {
diff --git a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
index cc1f13e..7acaf0d 100644
--- a/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
+++ b/server/lib/deltacloud/drivers/opennebula/opennebula_driver.rb
@@ -32,7 +32,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
   ######################################################################
   # Hardware profiles
   #####################################################################
-  def hardware_profiles(credentials, opts=nil)
+  def hardware_profiles(credentials, opts={})
     occi_client = new_client(credentials)
     xml = occi_client.get_instance_types
     if CloudClient.is_error?(xml)
@@ -70,8 +70,8 @@ class OpennebulaDriver < Deltacloud::BaseDriver
   ] ) unless defined?( REALMS )
 
 
-  def realms(credentials, opts=nil)
-    return REALMS if ( opts.nil? )
+  def realms(credentials, opts={})
+    return REALMS if ( opts.keys.empty? )
     results = REALMS
     results = filter_on( results, :id, opts )
     results
@@ -81,7 +81,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
   ######################################################################
   # Images
   ######################################################################
-  def images(credentials, opts=nil)
+  def images(credentials, opts={})
     occi_client = new_client(credentials)
 
     xml = treat_response(occi_client.get_images)
@@ -94,9 +94,9 @@ class OpennebulaDriver < Deltacloud::BaseDriver
     end
   end
 
-  def image(credentials, opts=nil)
+  def image(credentials, id)
     occi_client = new_client(credentials)
-    xml = treat_response(occi_client.get_image(opts[:id]))
+    xml = treat_response(occi_client.get_image(id))
     convert_image(xml, credentials)
   end
 
@@ -153,7 +153,7 @@ class OpennebulaDriver < Deltacloud::BaseDriver
     stopping.to(:finish)        .automatically
   end
 
-  def instances(credentials, opts=nil)
+  def instances(credentials, opts={})
     occi_client = new_client(credentials)
 
     xml = treat_response(occi_client.get_vms)
@@ -168,13 +168,13 @@ class OpennebulaDriver < Deltacloud::BaseDriver
     instances = filter_on( instances, :state, opts )
   end
 
-  def instance(credentials, opts=nil)
+  def instance(credentials, id)
     occi_client = new_client(credentials)
-    xml = treat_response(occi_client.get_vm(opts[:id]))
+    xml = treat_response(occi_client.get_vm(id))
     convert_instance(xml, credentials)
   end
 
-  def create_instance(credentials, image_id, opts=nil)
+  def create_instance(credentials, image_id, opts={})
     occi_client = new_client(credentials)
 
     storage_href = "#{occi_client.endpoint}/storage/#{image_id}"
diff --git a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
index ca1c236..4042f3e 100644
--- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
+++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
@@ -46,7 +46,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
     filter_hardware_profiles(results, opts)
   end
 
-  def images(credentials, opts=nil)
+  def images(credentials, opts={})
     rs = new_client(credentials)
     results = []
     safely do
@@ -65,7 +65,7 @@ class RackspaceDriver < Deltacloud::BaseDriver
   end
 
   #rackspace does not at this stage have realms... its all US/TX, all the time (at least at time of writing)
-  def realms(credentials, opts=nil)
+  def realms(credentials, opts={})
     [Realm.new( {
       :id=>"us",
       :name=>"United States",
diff --git a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
index 55eec11..bf6d561 100644
--- a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
+++ b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
@@ -26,7 +26,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
 
   feature :instances, :user_name
 
-  def images(credentails, opts=nil)
+  def images(credentails, opts={})
     safely do
       rh = RimuHostingClient.new(credentails)
       images = rh.list_images.map do | image |
@@ -61,7 +61,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
     filter_hardware_profiles(results, opts)
   end
 
-  def realms(credentials, opts=nil)
+  def realms(credentials, opts={})
     [Realm.new( {
             :id=>"rimu",
             :name=>"RimuHosting",
@@ -69,7 +69,7 @@ class RimuhostingDriver < Deltacloud::BaseDriver
     } )]
   end
 
-  def instances(credentials, opts=nil)
+  def instances(credentials, opts={})
     safely do
       rh = RimuHostingClient.new(credentials)
       instances = rh.list_nodes.map do | inst |
diff --git a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
index 3260c47..ba2fb04 100644
--- a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
+++ b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
@@ -54,7 +54,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>  "PENDING", "2" =>  "STOPPED", "4"
 # IMAGES
 #--
 #aka "vapp_templates"
-  def images(credentials, opts=nil)
+  def images(credentials, opts={})
       image_list = []
       terremark_client = new_client(credentials)
       safely do
@@ -78,7 +78,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>  "PENDING", "2" =>  "STOPPED", "4"
 # REALMS
 #--
 #only one realm... everything in US?
-  def realms(credentials, opts=nil)
+  def realms(credentials, opts={})
      [Realm.new( {
       :id=>"US-Miami",
       :name=>"United States - Miami",
@@ -90,7 +90,7 @@ VAPP_STATE_MAP = { "0" =>  "PENDING", "1" =>  "PENDING", "2" =>  "STOPPED", "4"
 # INSTANCES
 #--
 #aka vApps
-  def instances(credentials, opts=nil)
+  def instances(credentials, opts={})
       instances = []
       terremark_client = new_client(credentials)
       safely do
diff --git a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
index 253a2e5..7b1b1e4 100644
--- a/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
+++ b/server/lib/deltacloud/drivers/vsphere/vsphere_driver.rb
@@ -73,7 +73,7 @@ module Deltacloud::Drivers::Vsphere
 
     # Images are virtual machines with 'template' flag set to be true.
     # Thus we're getting them using find_vm and list_virtual_machines
-    def images(credentials, opts=nil)
+    def images(credentials, opts={})
       cloud = new_client(credentials)
       img_arr = []
       profiles = hardware_profiles(credentials)
@@ -123,7 +123,7 @@ module Deltacloud::Drivers::Vsphere
     end
 
     # List all datacenters managed by the vSphere or vCenter entrypoint.
-    def realms(credentials, opts=nil)
+    def realms(credentials, opts={})
       vsphere = new_client(credentials)
       safely do
         if opts and opts[:id]
@@ -140,7 +140,7 @@ module Deltacloud::Drivers::Vsphere
 
     # List all running instances, across all datacenters. DeltaCloud API does
     # not yet support filtering instances by realm.
-    def instances(credentials, opts=nil)
+    def instances(credentials, opts={})
       cloud = new_client(credentials)
       inst_arr, machine_vms, pending_vms = [], [], []
       safely do
diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb b/server/lib/deltacloud/helpers/deltacloud_helper.rb
index 4ad0be9..ef107c7 100644
--- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
+++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
@@ -79,7 +79,7 @@ module Deltacloud::Helpers
 
     def show(model)
       @benchmark = Benchmark.measure do
-        @element = driver.send(model, credentials, { :id => params[:id]} )
+        @element = driver.send(model, credentials, params[:id])
       end
       headers['X-Backend-Runtime'] = @benchmark.real.to_s
       instance_variable_set("@#{model}", @element)
@@ -105,7 +105,7 @@ module Deltacloud::Helpers
     end
 
     def instance_action(name)
-      original_instance = driver.instance(credentials, :id => params[:id])
+      original_instance = driver.instance(credentials, params[:id])
 
       # If original instance doesn't include called action
       # return with 405 error (Method is not Allowed)
diff --git a/server/tests/api/driver_test.rb b/server/tests/api/driver_test.rb
index c9facbf..2ec22ef 100644
--- a/server/tests/api/driver_test.rb
+++ b/server/tests/api/driver_test.rb
@@ -47,19 +47,19 @@ describe 'Deltacloud drivers API' do
 
   METHODS = {
     :firewalls => [[:credentials], [:opts, "{  }"]],
-    :firewall  => [[:credentials], [:opts, "{  }"]],
+    :firewall  => [[:credentials], [:id,]],
     :keys    => [[:credentials], [:opts, "{  }"]],
-    :key     => [[:credentials], [:opts]],
+    :key     => [[:credentials], [:id]],
     :storage_snapshots => [[:credentials], [:opts, "{  }"]],
-    :storage_snapshot  => [[:credentials], [:opts]],
+    :storage_snapshot  => [[:credentials], [:id]],
     :storage_volumes => [[:credentials], [:opts, "{  }"]],
-    :storage_volume  => [[:credentials], [:opts]],
+    :storage_volume  => [[:credentials], [:id]],
     :realms    => [[:credentials], [:opts, "{  }"]],
-    :realm     => [[:credentials], [:opts]],
+    :realm     => [[:credentials], [:id]],
     :images    => [[:credentials], [:opts, "{  }"]],
-    :image     => [[:credentials], [:opts]],
+    :image     => [[:credentials], [:id]],
     :instances => [[:credentials], [:opts, "{  }"]],
-    :instance  => [[:credentials], [:opts]],
+    :instance  => [[:credentials], [:id]],
     :create_instance => [[:credentials], [:image_id], [:opts, "{  }"]],
     :destroy_instance => [[:credentials], [:id]],
     :stop_instance => [[:credentials], [:id]],
@@ -71,7 +71,13 @@ describe 'Deltacloud drivers API' do
     METHODS.each do |m, definition|
       it "should have the correct parameters for the :#{m} method in #{key} driver" do
         next unless Deltacloud.new(key).backend.respond_to? m
-        Arguments.names(Deltacloud.new(key).backend.class, m).must_equal definition
+        is_optional = (definition[1][1] and definition[1][1] == '{  }')
+        if is_optional
+          Arguments.names(Deltacloud.new(key).backend.class, m)[1][1].wont_be_nil
+          Arguments.names(Deltacloud.new(key).backend.class, m)[1][1].must_equal '{  }'
+        else
+          Arguments.names(Deltacloud.new(key).backend.class, m)[1][1].must_be_nil
+        end
       end
     end
   end
diff --git a/server/tests/drivers/mock/instances_test.rb b/server/tests/drivers/mock/instances_test.rb
index d44fac5..0855190 100644
--- a/server/tests/drivers/mock/instances_test.rb
+++ b/server/tests/drivers/mock/instances_test.rb
@@ -261,6 +261,7 @@ describe 'Deltacloud API instances' do
       :image_id => image_id,
       :realm_id => realm_id,
     }
+    puts last_response.body
     last_response.status.must_equal 201 # HTTP_CREATED
     last_response.headers['Location'].wont_be_nil # Location header must be set, pointing to new the instance
     instance_id = last_response.headers['Location'].split('/').last
-- 
1.7.10.2