deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Fojtik ...@mifo.sk>
Subject Re: [PATCH core] Core: Prevent hash_capability to fail finding methods (DTACLOUD-265)
Date Fri, 10 Aug 2012 07:44:06 GMT
On Aug 10, 2012, at 12:55 AM, David Lutterkort <lutter@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



Mime
View raw message