[PATCH aeolus-conductor] BZ#753880 Catch exceptions on DC valid credentials request

Martyn Taylor mtaylor at redhat.com
Tue Feb 7 10:28:36 UTC 2012


On 02/06/2012 07:11 PM, Matt Wagner wrote:
> On Mon, Feb 06, 2012 at 05:58:44PM +0000, mtaylor at redhat.com wrote:
>> From: Martyn Taylor<mtaylor at redhat.com>
>>
>> ---
>>   src/app/models/provider_account.rb       |    8 ++++++--
>>   src/spec/models/provider_account_spec.rb |    8 ++++++++
>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb
>> index 89009aa..af107de 100644
>> --- a/src/app/models/provider_account.rb
>> +++ b/src/app/models/provider_account.rb
>> @@ -86,8 +86,12 @@ class ProviderAccount<  ActiveRecord::Base
>>     end
>>
>>     def validate_credentials
>> -    unless valid_credentials?
>> -      errors.add(:base, "Login Credentials are Invalid for this Provider")
>> +    begin
>> +      unless valid_credentials?
>> +        errors.add(:base, "Login Credentials are Invalid for this Provider")
>> +      end
>> +    rescue
>> +      errors.add(:base, "An error occured when checking provider credentials please check your setup and try again")
> This reads like a run-on sentence. "Please check your setup..." should
> begin a new sentence.
>
Sure
> Similarly, since this is Conductor, I think this should be in en.yml.

All error messages are defined in this way in conductor atm, I just 
followed suit.  I guess in the future though we need some effort in 
porting them all to en.ym
>>       end
>>     end
>>
>> diff --git a/src/spec/models/provider_account_spec.rb b/src/spec/models/provider_account_spec.rb
>> index 04b4b1e..49039f4 100644
>> --- a/src/spec/models/provider_account_spec.rb
>> +++ b/src/spec/models/provider_account_spec.rb
>> @@ -64,6 +64,14 @@ describe ProviderAccount do
>>       provider_account.save.should == false
>>     end
>>
>> +  it "should add errors when testing credentials fails" do
>> +    provider_account = Factory.build(:mock_provider_account)
>> +    provider_account.credentials_hash = {'password' =>  "wrong_password"}
>> +    provider_account.stub(:valid_credentials).and_raise("DeltacloudError")
>> +    provider_account.save.should == false
>> +    provider_account.errors[:base].should == ["An error occured when checking provider credentials please check your setup and try again"]
> This should be changed to reference en.yml if the above is changed.
>
>> +  end
>> +
>>     it "should fail to create a cloud account if fetching of hw profiles fails" do
>>       provider_account = Factory.build(:mock_provider_account)
>>       provider_account.stub(:populate_hardware_profiles).and_raise(ActiveRecord::RecordInvalid)
>> -- 
>> 1.7.6.4
> This looks good, but with two nits inline. Since you seem to have taken
> off and today is code freeze, I will make these changes myself, test,
> and then push.
Ahh great, thank you
> -- Matt




More information about the aeolus-devel mailing list