[PATCH aeolus-cli] BZ#797157 Validate aeolus config

Martyn Taylor mtaylor at redhat.com
Thu Mar 1 16:38:50 UTC 2012


On 03/01/2012 01:09 PM, Jan Provaznik wrote:
> On 02/24/2012 01:21 PM, mtaylor at redhat.com wrote:
>> From: Martyn Taylor<mtaylor at redhat.com>
>>
>> ---
>>   lib/aeolus_cli.rb                      |    2 ++
>>   lib/aeolus_cli/command/base_command.rb |   28 
>> ++++++++++++++++++++--------
>>   lib/aeolus_cli/exceptions.rb           |   28 
>> ++++++++++++++++++++++++++++
>>   spec/command/base_command_spec.rb      |    5 +++++
>>   spec/spec_helper.rb                    |    1 +
>>   5 files changed, 56 insertions(+), 8 deletions(-)
>>   create mode 100644 lib/aeolus_cli/exceptions.rb
>>
>> diff --git a/lib/aeolus_cli.rb b/lib/aeolus_cli.rb
>> index 8070845..2e6e88c 100644
>> --- a/lib/aeolus_cli.rb
>> +++ b/lib/aeolus_cli.rb
>> @@ -33,3 +33,5 @@ require File.join(File.dirname(__FILE__), 
>> 'aeolus_cli/model', 'provider')
>>   require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 
>> 'provider_account')
>>   require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 
>> 'provider_type')
>>   require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 
>> 'environment')
>> +
>> +require File.join(File.dirname(__FILE__), 'aeolus_cli/', 'exceptions')
>> diff --git a/lib/aeolus_cli/command/base_command.rb 
>> b/lib/aeolus_cli/command/base_command.rb
>> index 422e5f4..b9584d6 100644
>> --- a/lib/aeolus_cli/command/base_command.rb
>> +++ b/lib/aeolus_cli/command/base_command.rb
>> @@ -24,11 +24,15 @@ module Aeolus
>>         attr_accessor :options
>>
>>         def initialize(opts={}, logger=nil)
>> -        logger(logger)
>> -        @options = opts
>> -        @config_location = "~/.aeolus-cli"
>> -        @config = load_config
>> -        configure_active_resource
>> +        begin
>> +          logger(logger)
>> +          @options = opts
>> +          @config_location = "~/.aeolus-cli"
>> +          @config = load_config
>> +          configure_active_resource
>> +        rescue =>  e
>> +          handle_exception(e)
>> +        end
>>         end
>>
>>         protected
>> @@ -115,6 +119,10 @@ module Aeolus
>>             code = "Argument Error"
>>             message = e.message
>>
>> +        elsif e.is_a?(Aeolus::CLI::ConfigError)
>> +          code = "Config Error"
>> +          message = e.message + "; please check ~/.aeolus-cli"
>> +
>>           elsif e.respond_to?(:response)
>>             doc = Nokogiri::XML e.response.body
>>             code = doc.xpath("/error/code").text
>> @@ -167,9 +175,13 @@ module Aeolus
>>         end
>>
>>         def configure_active_resource
>> -        Aeolus::CLI::Base.site = @config[:conductor][:url]
>> -        Aeolus::CLI::Base.user = @config[:conductor][:username]
>> -        Aeolus::CLI::Base.password = @config[:conductor][:password]
>> +        if @config.has_key?(:conductor)&&  
>> @config[:conductor].keys.to_set.subset?([:url, :password, 
>> :username].to_set)
>> +          Aeolus::CLI::Base.site = @config[:conductor][:url]
>> +          Aeolus::CLI::Base.user = @config[:conductor][:username]
>> +          Aeolus::CLI::Base.password = @config[:conductor][:password]
>> +        else
>> +          raise Aeolus::CLI::ConfigError.new("Error in configuration 
>> file")
>> +        end
>
> If I comment out :url key, so it looks:
> :conductor:
>   #:url: http://localhost:3000/api
>   :username: admin
>   :password: password
>
> then I get:
> $ aeolus-image list --images
>
> ERROR:  Unknown Error => undefined method `path' for nil:NilClass
> /usr/lib/ruby/gems/1.8/gems/activeresource-3.0.10/lib/active_resource/base.rb:562:in 
> `prefix'
> ...
>
>
> Could this be extended to print which config key is missing? Something 
> like this:
>
> raise Aeolus::CLI::ConfigError.new("#{key} is missing") unless 
> @config.has_key?(:conductor)
> [:url, :password, :username].each do |key|
>   raise Aeolus::CLI::ConfigError.new("#{key} is missing") unless 
> @config[:conductor].has_key?(key)
> end
Good Idea I've updated and resent
>
>>         end
>>
>>         # Print Collection to STDOUT in a column layout
>> diff --git a/lib/aeolus_cli/exceptions.rb b/lib/aeolus_cli/exceptions.rb
>> new file mode 100644
>> index 0000000..a523266
>> --- /dev/null
>> +++ b/lib/aeolus_cli/exceptions.rb
>> @@ -0,0 +1,28 @@
>> +#
>> +#   Copyright 2011 Red Hat, Inc.
>> +#
>> +#   Licensed under the Apache License, Version 2.0 (the "License");
>> +#   you may not use this file except in compliance with the License.
>> +#   You may obtain a copy of the License at
>> +#
>> +#       http://www.apache.org/licenses/LICENSE-2.0
>> +#
>> +#   Unless required by applicable law or agreed to in writing, software
>> +#   distributed under the License is distributed on an "AS IS" BASIS,
>> +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
>> implied.
>> +#   See the License for the specific language governing permissions and
>> +#   limitations under the License.
>> +#
>> +module Aeolus
>> +  module CLI
>> +    class Error<  StandardError
>> +      attr_reader :message
>> +
>> +      def initialize(message)
>> +        @message  = message
>> +      end
>> +    end
>
> Any reason for Error definition? Isn't sufficient this?
>
> class ConfigError<  StandardError; end

Yes, Aeouls::CLI::Error is the default Error within this module.  It 
also overrides the message accessor and the constructor to only allow 
messages created with message, and set this to readable only.

In the future we might extend Aeolus::CLI::Error to have an Error code.

>
>> +
>> +    class ConfigError<  Error; end
>> +  end
>> +end
>> \ No newline at end of file
>> diff --git a/spec/command/base_command_spec.rb 
>> b/spec/command/base_command_spec.rb
>> index 8c1ef5f..f71f91a 100644
>> --- a/spec/command/base_command_spec.rb
>> +++ b/spec/command/base_command_spec.rb
>> @@ -26,6 +26,11 @@ module Aeolus
>>             Aeolus::CLI::Base.user.to_s.should == "admin"
>>             Aeolus::CLI::Base.password.to_s.should == "password"
>>           end
>> +
>> +        it "should raise error on invalid config" do
>> +          base_command.instance_eval { @config = {} }
>> +          lambda { base_command.send(:configure_active_resource) 
>> }.should raise_error(Aeolus::CLI::ConfigError, "Error in 
>> configuration file")
>> +        end
>>         end
>>
>>         describe "#read_file" do
>> diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
>> index d198732..368c18f 100644
>> --- a/spec/spec_helper.rb
>> +++ b/spec/spec_helper.rb
>> @@ -14,6 +14,7 @@
>>
>>   $:<<  File.expand_path(File.join(File.dirname(__FILE__), 
>> "../lib/aeolus_cli/command/"))
>>   $:<<  File.expand_path(File.join(File.dirname(__FILE__), 
>> "../lib/aeolus_cli/model/"))
>> +$:<<  File.expand_path(File.join(File.dirname(__FILE__), 
>> "../lib/aeolus_cli/"))
>>
>>   require 'aeolus_cli'
>>   require 'stringio'
>
> Hi Martyn,
> couple of minor notes inline.
>
> Jan
Thanks

Martyn




More information about the aeolus-devel mailing list