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/08/08 11:24:35 UTC
[PATCH core] Core: Prevent hash_capability to fail finding methods (DTACLOUD-265)
From: Michal Fojtik <mf...@redhat.com>
* When the driver inherits its methods from other driver,
the has_capability failed to find them, which cause to
not advertise them.
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/deltacloud/drivers/base_driver.rb | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/server/lib/deltacloud/drivers/base_driver.rb b/server/lib/deltacloud/drivers/base_driver.rb
index 21e4382..30d0f92 100644
--- a/server/lib/deltacloud/drivers/base_driver.rb
+++ b/server/lib/deltacloud/drivers/base_driver.rb
@@ -157,7 +157,11 @@ module Deltacloud
def has_capability?(method)
method = (RUBY_VERSION =~ /^1\.9/) ? method : method.to_s
- (self.class.instance_methods - self.class.superclass.instance_methods).include? method
+ # Prevent has_capability fail when driver is inherited from another
+ # driver, like Eucalyptus
+ superclass_methods = self.class.superclass.name == 'Deltacloud::BaseDriver' ?
+ self.class.superclass.instance_methods : []
+ (self.class.instance_methods - superclass_methods).include? method
end
## Capabilities
--
1.7.10.2
Re: [PATCH core] Core: Prevent hash_capability to fail finding
methods (DTACLOUD-265)
Posted by David Lutterkort <lu...@redhat.com>.
On Fri, 2012-08-10 at 09:44 +0200, Michal Fojtik wrote:
> Yes, I was thinking about two possible future solutions:
>
> 1. Remove all images(), instances(), etcS() methods from the BaseDriver
I don't see any of the plural methods in BaseDriver (images, realms,
etc.)
> Honestly, I don't really like the fact, we perform this capability checking
> in two places. But for now, I think we can fix it using this patch, then
> discuss how to improve it in overall.
Yes, by all means, commit this patch.
David
Re: [PATCH core] Core: Prevent hash_capability to fail finding methods (DTACLOUD-265)
Posted by Michal Fojtik <mi...@mifo.sk>.
On Aug 10, 2012, at 12:55 AM, David Lutterkort <lu...@redhat.com> wrote:
>> diff --git a/server/lib/deltacloud/drivers/base_driver.rb b/server/lib/deltacloud/drivers/base_driver.rb
>> index 21e4382..30d0f92 100644
>> --- a/server/lib/deltacloud/drivers/base_driver.rb
>> +++ b/server/lib/deltacloud/drivers/base_driver.rb
>> @@ -157,7 +157,11 @@ module Deltacloud
>>
>> def has_capability?(method)
>> method = (RUBY_VERSION =~ /^1\.9/) ? method : method.to_s
>> - (self.class.instance_methods - self.class.superclass.instance_methods).include? method
>> + # Prevent has_capability fail when driver is inherited from another
>> + # driver, like Eucalyptus
>> + superclass_methods = self.class.superclass.name == 'Deltacloud::BaseDriver' ?
>
> Why compare by name here instead of self.class.superclass ==
> Deltacloud::BaseDriver ?
Good point, I'll change it ;-)
> Why is has_capability? not just an alias for respond_to? ? Where we use
Because respond_to? will always return 'true', since all driver methods
are inherited from the BaseDriver class. It means, when XXXDriver does
not implement images() method, then the BaseDriver 'image()' method
can't work properly. And since all the resourceS() methods are defined
already in BaseDriver, the respond_to? will not be useful here :(
> it, it seems, we only check for methods that aren't defined in the
> base_driver anyay. So maybe we don't need all these superclass
> complications.
Yes, I was thinking about two possible future solutions:
1. Remove all images(), instances(), etcS() methods from the BaseDriver
2. Perform capability checking just by Rabbit (with_capability on operation)
Honestly, I don't really like the fact, we perform this capability checking
in two places. But for now, I think we can fix it using this patch, then
discuss how to improve it in overall.
-- Michal
Re: [PATCH core] Core: Prevent hash_capability to fail finding
methods (DTACLOUD-265)
Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2012-08-08 at 11:24 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
>
> * When the driver inherits its methods from other driver,
> the has_capability failed to find them, which cause to
> not advertise them.
>
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
> server/lib/deltacloud/drivers/base_driver.rb | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
ACK; minor nit:
> diff --git a/server/lib/deltacloud/drivers/base_driver.rb b/server/lib/deltacloud/drivers/base_driver.rb
> index 21e4382..30d0f92 100644
> --- a/server/lib/deltacloud/drivers/base_driver.rb
> +++ b/server/lib/deltacloud/drivers/base_driver.rb
> @@ -157,7 +157,11 @@ module Deltacloud
>
> def has_capability?(method)
> method = (RUBY_VERSION =~ /^1\.9/) ? method : method.to_s
> - (self.class.instance_methods - self.class.superclass.instance_methods).include? method
> + # Prevent has_capability fail when driver is inherited from another
> + # driver, like Eucalyptus
> + superclass_methods = self.class.superclass.name == 'Deltacloud::BaseDriver' ?
Why compare by name here instead of self.class.superclass ==
Deltacloud::BaseDriver ?
Why is has_capability? not just an alias for respond_to? ? Where we use
it, it seems, we only check for methods that aren't defined in the
base_driver anyay. So maybe we don't need all these superclass
complications.
David