[PATCH conductor 1/3] BZ 768089: Refactor provider account matching to facilitate stubbing

Scott Seago sseago at redhat.com
Tue Dec 20 15:52:54 UTC 2011


On 12/20/2011 10:31 AM, Scott Seago wrote:
> On 12/20/2011 05:33 AM, Jan Provaznik wrote:
>> On 12/20/2011 07:13 AM, Scott Seago wrote:
>>
>>> diff --git a/src/app/models/provider_account.rb 
>>> b/src/app/models/provider_account.rb
>>> index 88ac8d4..0ba423b 100644
>>> --- a/src/app/models/provider_account.rb
>>> +++ b/src/app/models/provider_account.rb
>>> @@ -338,6 +338,37 @@ class ProviderAccount<  ActiveRecord::Base
>>>       end
>>>     end
>>>
>>> +  def instance_matches(instance, matched, errors)
>>> +    if !provider.enabled?
>>> +      errors<<  I18n.t('instances.errors.must_be_enabled', 
>>> :account_name =>  name)
>>> +    elsif quota.reached?
>>> +      errors<<  
>>> I18n.t('instances.errors.provider_account_quota_reached', 
>>> :account_name =>  name)
>>> +    # match_provider_hardware_profile returns a single provider
>>> +    # hardware_profile that can satisfy the input hardware_profile
>>> +    elsif !(hwp = 
>>> HardwareProfile.match_provider_hardware_profile(provider, 
>>> instance.hardware_profile))
>>> +      errors<<  
>>> I18n.t('instances.errors.hw_profile_match_not_found', :account_name 
>>> =>  name)
>>> +    elsif (account_images = 
>>> Instance.provider_images_for_match(instance,provider)).empty?
>>> +      errors<<  
>>> I18n.t('instances.errors.image_not_pushed_to_provider', 
>>> :account_name =>  name)
>>> +    elsif instance.requires_config_server? and config_server.nil?
>>> +      errors<<  
>>> I18n.t('instances.errors.no_config_server_available', :account_name 
>>> =>  name)
>>> +    else
>>
>> I like multiple separate 'ifs' more - a user gets all errors 
>> together, so for example he doesn't have to spend time with pushing 
>> an image to a provider because quota is reached and he can't launch 
>> an instance anyway.
>>
> Sure -- perhaps I can refactor this too. Again, I was preserving the 
> previous behavior where the code was exiting the loop on error. I'll 
> revisit when I make the above fixes.
Actually, I think this is best as-is. These aren't normal "form 
submission" errors -- 'errors' are really 'reasons no match is found' -- 
so once there's no match, there's no reason to go on. "This provider 
account doesn't match because the provider isn't enabled", "This 
provider doesn't match because there's no matching HWP"

If there are 5 provider accounts, and there are 2 matches, the "reasons 
not to match" array gets reasons for 3 of the 5, but as long as the 
matches list is not empty, we throw the reasons/errors away. If there 
are no matches, the errors array gives the reason we failed matching for 
each provider account.

Scott




More information about the aeolus-devel mailing list