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

Matt Wagner matt.wagner at redhat.com
Mon Feb 6 19:11:11 UTC 2012


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.

Similarly, since this is Conductor, I think this should be in en.yml.

>      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.

-- Matt



More information about the aeolus-devel mailing list